-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Branch fix: No infinite loops during metadata requests, invalidate metadata more, exception hierarchy #100
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
Conversation
@rdiomar - can you merge this in? I'm not a repo collaborator... |
Good point - added that exception too |
That push should address your comments (I made a method our of setUp() to make clear what it is actually doing). |
I understand that setUp is making sure we can create topics, but it's still not actually setting up anything. It's testing topic creation, which I believe belongs in a test method. The topic it creates is not used anywhere else, and it has no other side effects. |
@rdiomar it is creating a topic of name "self.id()[self.id().rindex(".")+1:]", which matches the name of each test method (e.g. test_multi_process_consumer). The majority of the test methods assume that producers/consumers can immediately access their topic, hence the necessity of warming the topic. |
Ah I see! Sorry I didn't get that earlier. That's also why you changed the topic names. How about this:
Then in the tests, always use self.topic instead of a string that matches the test name. |
Good idea, I've submitted an update (that also adds a bit of randomness to the topic name). |
Branch fix: No infinite loops during metadata requests, invalidate metadata more, exception hierarchy
Thanks @cosbynator! |
thank you both @cosbynator and @rdiomar ! 👍 |
This is an update to #91 that properly adds my new commit on top of the upstream changes (I had accidentally performed a rebase instead of merge that put my branch in a bad state).