Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set the etcd max transaction limit to a reasonable default #200

Closed

Conversation

nitinprakash96
Copy link

No description provided.

@purpleidea
Copy link
Owner

@nitinprakash96 Thanks for the patch! This value should be a constant somewhere at the top of the file, and then used here.

You'll also need your commit messages to start with etcd: The commit message for this to pass.

Thanks!

@purpleidea
Copy link
Owner

@nitinprakash96 I have stupidly forgotten that this patch depends on a newer version of etcd, and we are currently vendoring an older one. As a result, it can't be merged until we bump etcd. I can cherry-pick it and merge down the road if you're interested.

If so, please make sure you run go fmt on your code (make gofmt will fix it) and rebase everything into a single patch. (ping if you need tips on how to do this). Lastly the comment should be on the constant, not the usage.

And your patch failed because commit messages need to be capitalized. Eg: etcd: Set the not etcd: set the

Thanks!

@nitinprakash96
Copy link
Author

@purpleidea Ah I see now why the patch fails to build! I'll rebase and wait for further instructions.

@nitinprakash96
Copy link
Author

The build fails possibly due to the older version of etcd the we're using. Is that so @purpleidea ?

@purpleidea
Copy link
Owner

purpleidea commented Jul 5, 2017 via email

@nitinprakash96
Copy link
Author

Okay so I tried running the tests manually on my system and apparently all of the fail is caused by the older version. For example, when running gometalinter,

gometalinter --disable-all --enable=goimports --enable=golint --enable=gotype --enable=misspell --enable=unconvert .

outputs:

etcd/etcd.go:1660:2:warning: invalid operation: cfg (variable of type *github.com/purpleidea/mgmt/vendor/github.com/coreos/etcd/embed.Config) has no field or method MaxTxnOps (unconvert)

or while running go test,

go test github.com/purpleidea/mgmt

outputs:

# github.com/purpleidea/mgmt/etcd
etcd/etcd.go:1660: cfg.MaxTxnOps undefined (type *embed.Config has no field or method MaxTxnOps)

Other than that there was just gofmt which I fixed as per your previous suggestion. But still if there's any solution could you help me out?

@purpleidea
Copy link
Owner

purpleidea commented Jul 6, 2017 via email

@nitinprakash96
Copy link
Author

I'll take a look into other love issues and see if I can fix anything.

@purpleidea
Copy link
Owner

@nitinprakash96 Sorry for the delay... If you'd like to rebase your patch, it should work now :)

Also I'd recommend removing the uint() wrapper from the constant. If needed add it below where the value is actually used!

Thanks!

@purpleidea
Copy link
Owner

Turns out I'm stupid and this isn't in a released version yet... So we'll have ti wait for etcd-io/etcd@ae7ddfb to be in a released version.

@purpleidea
Copy link
Owner

Fixed in 24cb2e6

@purpleidea purpleidea closed this Feb 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants