Skip to content

feat: Implement committer#13

Merged
dpcollins-google merged 4 commits into
masterfrom
committer
Sep 14, 2020
Merged

feat: Implement committer#13
dpcollins-google merged 4 commits into
masterfrom
committer

Conversation

@dpcollins-google

Copy link
Copy Markdown
Contributor

Also small fix to retrying connection so it doesn't leak reads/writes from previous connections.

Also small fix to retrying connection so it doesn't leak reads/writes from previous connections.
@dpcollins-google dpcollins-google requested a review from pradn August 13, 2020 15:20

class Committer(AsyncContextManager):
"""
A Committer is able to commit subscribers completed offsets.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

super nit: "subscribers' "?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment thread google/cloud/pubsublite/internal/wire/committer_impl.py
Comment thread google/cloud/pubsublite/internal/wire/committer_impl.py Outdated
await self._stop_loopers()
await connection.write(StreamingCommitCursorRequest(initial=self._initial))
response = await connection.read()
if "initial" not in response:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ideally we're not doing substrings to check for errors? This happens in a few places.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This isn't substrings.... its checking for a subfield in the response. This is how you're supposed to check for oneof field presence in the new proto lib.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah gotcha!

Comment thread google/cloud/pubsublite/internal/wire/connection_reinitializer.py
Comment thread setup.py
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 14, 2020
@dpcollins-google dpcollins-google merged commit aa9aca8 into master Sep 14, 2020
@anguillanneuf anguillanneuf deleted the committer branch March 25, 2022 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

2 participants