-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Make capacity/validity/updateinterval/activeupdate for Auth Caches configurable via nodetool #1370
Make capacity/validity/updateinterval/activeupdate for Auth Caches configurable via nodetool #1370
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits but nothing major. Looks good.
probe.output().out.println("Active Update: " + authCacheMBean.getActiveUpdate()); | ||
} | ||
|
||
private AuthCacheMBean getAuthCacheMBean(NodeProbe probe, String cacheName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a copy of this in both GetAuthCacheConfig and SetAuthCacheConfig. Is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I moved this method to NodeProbe
. Now it is re-used across both commands.
|
||
tool = ToolRunner.invokeNodetool("getauthcacheconfig", "--cache-name"); | ||
assertThat(tool.getExitCode()).isEqualTo(1); | ||
assertThat(tool.getStdout()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: these line breaks make this test less readable. Code style guides have a soft recommendation at 120 char wide (see: https://cassandra.apache.org/_/development/code_style.html), specifically:
Try to keep lines under 120 characters, but use good judgement. It is better to exceed 120 by a little, than split a line that has no natural splitting points.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with you, having this on a single line seems to be more readable. Adjusted the line breaks.
29fd7c0
to
7c9664c
Compare
…nfigurable via nodetool patch by Aleksei Zotov; reviewed by Josh McKenzie for CASSANDRA-17063
7c9664c
to
8e9082e
Compare
patch by Aleksei Zotov; reviewed by Josh McKenzie for CASSANDRA-17063