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

Implements v1 of {Create,Describe,Delete}AclRequest #1236

Merged
merged 1 commit into from
Feb 22, 2019

Conversation

Mongey
Copy link
Contributor

@Mongey Mongey commented Dec 12, 2018

No description provided.

@Mongey Mongey force-pushed the cm-v1-create-acl-request branch from c23fdee to e8436b8 Compare December 18, 2018 20:19
@Mongey Mongey changed the title WIP: Implements v1 of {Create,Describe,Delete}AclRequest Implements v1 of {Create,Describe,Delete}AclRequest Dec 18, 2018
@bai bai requested review from varun06 and sam-obeid February 19, 2019 13:41
@@ -1,6 +1,7 @@
package sarama

type CreateAclsRequest struct {
Version int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Version has to be int, if we are casting it to/fro int16?

acl_types.go Show resolved Hide resolved
@Mongey Mongey force-pushed the cm-v1-create-acl-request branch from 97d687c to 2082771 Compare February 19, 2019 18:54
@Mongey Mongey force-pushed the cm-v1-create-acl-request branch from 2082771 to 66abce6 Compare February 19, 2019 19:13
@bai
Copy link
Contributor

bai commented Feb 22, 2019

Thanks @Mongey and @varun06 — as always 🙌

@bai bai merged commit 8c2d15c into IBM:master Feb 22, 2019
@Mongey Mongey deleted the cm-v1-create-acl-request branch April 14, 2019 14:52
@symaras
Copy link
Contributor

symaras commented Aug 6, 2019

It seems that the ClusterAdmin interface is not updated to cover the prefixed resources. The ListAcls method does not take a version argument. Thus, it creates a DescribeAclsRequest struct with no version defined and this defaults to V0_11_0_0.

Or do I miss something here? Let me know, I am happy to submit a PR for this.

thanks!

@symaras
Copy link
Contributor

symaras commented Aug 6, 2019

Opened an issue: #1451

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.

4 participants