-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Refactor kinesis connector #9644
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
dec5f9e to
f71f6f8
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.
The changes here are only removal of the shard_id parameter of process records, and migration to the cleanups fixture instead of try: finally:. The changed lines within that block are just indentation changes.
701e19a to
c8218c1
Compare
…t, prep work for bigger refactoring
…own to get better information about thread cleanup
3051a44 to
cf4317a
Compare
|
Discussed with alex, and decided to go with patch for now. Up for discussion though, if a reviewer thinks it should be minor :) |
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.
I have no strong opinions on the kinesis connector refactoring, but I think this is a step in the right direction. Anything to properly manage the lifetimes of the kinesis connector resources is a good thing!
edit: happy with semver patch
Motivation
Our
kinesis_connector.pyis currently used at several locations in our code, and the usage of it is in principal valid.However, it has some problems with threads not shutting down, processes kept alive indefinitely, and a lot of unused configuration options cluttering the code.
To continue using it, we want:
Changes
AWS::KinesisAnalytics::Application