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 shutdown requested implemention in python client. #53

Merged
merged 4 commits into from
Jun 23, 2017

Conversation

mikramulhaq
Copy link
Contributor

@pfifer - Starting this PR to add capability in python client to handle shutdown requested call. Added for java in PR awslabs/amazon-kinesis-client#174

:type checkpointer: amazon_kclpy.kcl.Checkpointer
:param checkpointer: A checkpointer which accepts a sequence number or no parameters.
'''
raise NotImplementedError
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably do nothing. If someone upgrades to the newer version without implementing the shutdownRequested it would cause their application to fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally this should be placed on the v2 version of the record processor in v2/processor.py

@@ -220,6 +220,17 @@ def shutdown(self, checkpointer, reason):
'''
raise NotImplementedError

@abc.abstractmethod
def shutdownRequested(self, checkpointer):
Copy link
Contributor

Choose a reason for hiding this comment

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

This should follow Python PEP8 style e.g. shutdown_requested.

For the v2 interface this should also use intermediate type, even though I did that wrong with the interface in the Java KCL. See v2/processor.py#L51 for an example

@pfifer
Copy link
Contributor

pfifer commented Jun 22, 2017

Thanks for the updates, one final thing:

Please confirm that we can use, modify, copy, and redistribute this contribution.

Thanks.

@mikramulhaq
Copy link
Contributor Author

@pfifer - Yes I approve that you can use, modify, copy and redistribute this contribution.

Thanks,
-ikram

@pfifer pfifer added this to the v1.4.5 milestone Jun 23, 2017
@pfifer pfifer merged commit 1e47120 into awslabs:master Jun 23, 2017
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.

2 participants