-
Notifications
You must be signed in to change notification settings - Fork 11
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
Update kafka-python to 2.0.2 to add support for python 3.8 #16
Conversation
8f32f6d
to
02c91a4
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.
Please bump the version in pack.yaml to 1.1.0
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.
Once pack.yaml is updated and the action returns a dict, we can merge this.
Addresses PR feedback (produce action should return a dict).
actions/produce.py
Outdated
try: | ||
assert topic in consumer.topics() | ||
except AssertionError as exc: | ||
raise AssertionError(f"{topic} does not exist.") from exc |
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.
This is not a direct replacement for ensure_topic_exists()
because: if the broker is configured to create topics automatically, then that will no longer happen here. Maybe this should actually just be a major version bump.
Also, apparently we have to update the sensors too.
This is a major version bump as topics will no longer be auto-created by the sensors or by the produce action even if the Kafka server is configured to auto-create the topics and partitions. That's a side-effect of updating a couple of major versions of the kafka-python dependency.
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.
Finally. CI is green
Closes #15
Closes #18