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 Close() OffsetManager interface #518

Merged
merged 1 commit into from Aug 19, 2015
Merged

Conversation

wvanbergen
Copy link
Contributor

This way, an OffsetManager that uses an external data store can properly clean up the resources it uses.

  • Should we add AsyncClose() for consistency?
  • I added a dummy implementation for the Kafka offset managers. Should be do something there now the function is present?

@eapache

@eapache
Copy link
Contributor

eapache commented Aug 19, 2015

Should we add AsyncClose() for consistency?

Does this make sense as a useful function for the ZK-based manager?

@eapache
Copy link
Contributor

eapache commented Aug 19, 2015

I added a dummy implementation for the Kafka offset managers. Should be do something there now the function is present?

Nope. The consumer is in a similar situation - all it does is (potentially) close the underlying client, but we don't even have that to deal with because the only constructor we provide is OffsetManagerFromClient.

@eapache
Copy link
Contributor

eapache commented Aug 19, 2015

Add AsyncClose if you think it's needed, then LGTM.

@wvanbergen
Copy link
Contributor Author

I don't think we'll need AsyncClose, because there are no channels to wait for.

wvanbergen added a commit that referenced this pull request Aug 19, 2015
Add Close() OffsetManager interface
@wvanbergen wvanbergen merged commit de8e312 into offset-manager Aug 19, 2015
@wvanbergen wvanbergen deleted the offset-manager-close branch August 19, 2015 17:19
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