-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
auth_command: create etcdctl auth status
command
#11536
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11536 +/- ##
==========================================
- Coverage 64.25% 63.66% -0.59%
==========================================
Files 403 403
Lines 38091 38116 +25
==========================================
- Hits 24475 24268 -207
- Misses 11979 12204 +225
- Partials 1637 1644 +7
Continue to review full report at Codecov.
|
d060265
to
2ce580e
Compare
I have worked further on what I believe is a mostly working command. I don't have a good idea on how to remove the Thanks.
EDIT: I have figured it out. It no longer has a |
@tarcinil thanks for the PR! I'd like to review it. But before that could you reorganize your commits with below rules for easier review?
|
131758a
to
fed0a0f
Compare
@mitake Let me know if that is the change to the commit that you had in mind. |
etcdctl auth status
command
Forcing Travis Build as the test that failed didn't have to do with my work and they passed before squash. Brittle test? |
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.
Could you squash 1st, 2nd, 4th and 5th commits into a single commit? The commit log should be *: add a new API and command for checking auth status
.
Also you can drop 3rd commit because authStore.IsAuthEnabled()
can be reused for the purpose and the method already has its tests.
Thanks a lot for your PR!
fed0a0f
to
deb850b
Compare
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.
935b0fe
to
98c7290
Compare
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.
Overall looks good. Thanks! We also need to highlight this change in CHANGELOG-3.5.
98c7290
to
a275392
Compare
a275392
to
cdf9368
Compare
lgtm |
@tarcinil can you add this to the changelog? |
LGTM. Thanks! |
I will look at doing the changelog tonight. |
This changes have started at etcdctl under auth.go, and make changes to stub out everything down into the internal raft. Made changes to the .proto files and regenerated them so that the local version would build successfully.
cdf9368
to
3d74793
Compare
@xiang90 Changelog has been added. I created an entry for etcdctl and API as it technically covers both. |
lgtm |
This changes have started at etcdctl under auth.go, and make changes to stub out everything down into the internal raft. Made changes to the .proto files and regenerated them so that the local version would build successfully.
fixes #11516