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

Add AdminClient.list_groups API #948

Merged
merged 7 commits into from
Sep 9, 2020
Merged

Add AdminClient.list_groups API #948

merged 7 commits into from
Sep 9, 2020

Conversation

messense
Copy link
Contributor

@messense messense commented Sep 7, 2020

Closes #223

Not sure how to add test cases for this feature.

@ghost
Copy link

ghost commented Sep 7, 2020

It looks like @messense hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

@messense
Copy link
Contributor Author

messense commented Sep 7, 2020

[clabot:check]

@ghost
Copy link

ghost commented Sep 7, 2020

@confluentinc It looks like @messense just signed our Contributor License Agreement. 👍

Always at your service,

clabot

Copy link
Contributor

@edenhill edenhill left a comment

Choose a reason for hiding this comment

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

Great stuff!

confluent_kafka/src/Metadata.c Show resolved Hide resolved
confluent_kafka/src/Metadata.c Show resolved Hide resolved
confluent_kafka/src/Metadata.c Show resolved Hide resolved
confluent_kafka/src/Metadata.c Outdated Show resolved Hide resolved
confluent_kafka/src/Metadata.c Outdated Show resolved Hide resolved
confluent_kafka/src/Metadata.c Outdated Show resolved Hide resolved
confluent_kafka/src/Metadata.c Outdated Show resolved Hide resolved
confluent_kafka/src/Metadata.c Outdated Show resolved Hide resolved
examples/adminapi.py Outdated Show resolved Hide resolved
tests/test_Admin.py Show resolved Hide resolved
:ivar str client_id: Client id
:ivar str client_host: Client hostname
:ivar bytes metadata: Member metadata(binary), format depends on protocol type
:ivar bytes assignment: Member assignment(binary), format depends on protocol type
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be great to have APIs to parse metadata and assignment.

cc confluentinc/librdkafka#2643

Copy link
Contributor

Choose a reason for hiding this comment

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

That'd be great, but could be provided as a separate python utility package, perhaps auto-generated from the AK json protocol specs.

Choose a reason for hiding this comment

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

I'm interested in decoding this data... is this work in progress somewhere? Or maybe point me to the spec you're referring to (I couldn't find it with limited googling) and I can take a crack at it.

Choose a reason for hiding this comment

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

Copy link

@JjudeGotLemon JjudeGotLemon May 26, 2021

Choose a reason for hiding this comment

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

@snagafritz What's your solution to decode it? Could you share it out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

def parse_member_assignment(buf):
    '''Parse Kafka group assignment'''
    import struct
    assigns = []
    if not buf:
        return assigns
    index = struct.calcsize('>hi')
    _version, assign_len = struct.unpack('>hi', buf[0:index])
    for _ in range(assign_len):
        size = struct.calcsize('>h')
        str_len = struct.unpack('>h', buf[index:index + size])[0]
        index += size
        topic = buf[index: index + str_len].decode('utf-8')
        index += str_len;
        size = struct.calcsize('>i')
        part_len = struct.unpack('>i', buf[index:index + size])[0]
        index += size
        parts = []
        for _ in range(part_len):
            part = struct.unpack('>i', buf[index:index + size])[0]
            parts.append(part)
            index += size
        assigns.append((topic, parts))
    return assigns

Choose a reason for hiding this comment

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

@messense Very helpful! Thanks

@messense messense requested a review from edenhill September 9, 2020 02:43
Copy link
Contributor

@edenhill edenhill left a comment

Choose a reason for hiding this comment

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

Solid stuff!
I think it would be useful to print the protocol and type in the example.
After that we're good to go

examples/adminapi.py Outdated Show resolved Hide resolved
groups = set(group.id for group in a.list_groups(group1))
assert group1 in groups, "Consumer group {} not found".format(group1)
groups = set(group.id for group in a.list_groups(group2))
assert group2 in groups, "Consumer group {} not found".format(group2)
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

:ivar str client_id: Client id
:ivar str client_host: Client hostname
:ivar bytes metadata: Member metadata(binary), format depends on protocol type
:ivar bytes assignment: Member assignment(binary), format depends on protocol type
Copy link
Contributor

Choose a reason for hiding this comment

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

That'd be great, but could be provided as a separate python utility package, perhaps auto-generated from the AK json protocol specs.

@@ -25,7 +25,8 @@

SCHEMA_REGISTRY_REQUIRES = ['requests']

AVRO_REQUIRES = ['fastavro>=0.23.0',
AVRO_REQUIRES = ['fastavro>=0.23.0,<1.0;python_version<"3.0"',
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this.

@messense messense requested a review from edenhill September 9, 2020 07:21
@edenhill edenhill merged commit 7768191 into confluentinc:master Sep 9, 2020
@edenhill
Copy link
Contributor

edenhill commented Sep 9, 2020

Great work on this, thank you!

@messense messense deleted the list-groups branch September 9, 2020 13:47
@RoyAtanu
Copy link

@edenhill Is there a timeline when this change will make to a release?

@edenhill
Copy link
Contributor

We'll release v1.6.0 after the holidays.

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.

ListGroups, DescribeGroups support (for ConsumerOffsetChecker type tool)
5 participants