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

Consumer offsets #98

Merged
merged 9 commits into from
Aug 21, 2019
Merged

Consumer offsets #98

merged 9 commits into from
Aug 21, 2019

Conversation

twm
Copy link
Collaborator

@twm twm commented Aug 14, 2019

  • Revert a backwards-incompatible change that was made as part of the coordinated consumer support. I'm not sure why this was done, as ConsumerGroup doesn't actually look at the result of stopping its consumers.
  • Add attributes that expose some of the consumer's offset information. These will be useful as metrics.

@twm twm requested a review from a team August 14, 2019 21:56
Otherwise snappy won't build.

Signed-off-by: Tom Most <tmost@blueplanet.com>
Signed-off-by: Tom Most <tmost@blueplanet.com>
Signed-off-by: Tom Most <tmost@blueplanet.com>
This makes them show up once at the same level as methods, rather than
twice (once with the ivars and again, undocumented, with the methods).

Signed-off-by: Tom Most <tmost@blueplanet.com>
Signed-off-by: Tom Most <tmost@blueplanet.com>
Signed-off-by: Tom Most <tmost@blueplanet.com>
It is bytes, not str.

Signed-off-by: Tom Most <tmost@blueplanet.com>
Copy link

@ryban ryban left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor comments.

@@ -43,6 +43,9 @@
class TestAfkakConsumer(unittest.SynchronousTestCase):
maxDiff = None

def assertNone(self, value):
Copy link

Choose a reason for hiding this comment

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

Why not use assertIsNone?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It doesn't appear in the SynchronousTestCase API documentation, which is where I looked. It does seem to exist, though. I will switch, thanks!

afkak/consumer.py Outdated Show resolved Hide resolved
Signed-off-by: Tom Most <tmost@blueplanet.com>
Signed-off-by: Tom Most <tmost@blueplanet.com>
@twm twm merged commit 9c22b60 into master Aug 21, 2019
@twm twm deleted the consumer-offsets branch August 21, 2019 01:43
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.

None yet

2 participants