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.

Sign up for GitHub

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

Jump to bottom

#798: Add support for auto-acknowledging pulled messages. #1637

Closed
wants to merge 2 commits into from
Closed

#798: Add support for auto-acknowledging pulled messages. #1637

wants to merge 2 commits into from

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Mar 18, 2016

Follows @jgeewax's suggested implementation.

Closes #798.

Hold off merging until comparing with the alternate implementation suggested by @tmatsuo (#1636).

@tseaver tseaver added do not merge Indicates a pull request not ready for merge, due to either quality or timing. api: pubsub Issues related to the Pub/Sub API. labels Mar 18, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 18, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 3d1bb30 on tseaver:798-jgeewax_strategy into 98edc64 on GoogleCloudPlatform:master.

@tseaver
Copy link
Contributor Author

tseaver commented Mar 18, 2016

Ugh, when did @coveralls turn back into a noisy commenter?

@tseaver tseaver mentioned this pull request Mar 18, 2016
Merged
@tseaver
Copy link
Contributor Author

tseaver commented Mar 18, 2016

Ugh, when did @coveralls turn back into a noisy commenter?

I just disabled @coveralls commenting on the project page.

@dhermes
Copy link
Contributor

dhermes commented Mar 18, 2016

How did you disable it? (Primarily I want to make sure the webhook is still active.)

@dhermes
Copy link
Contributor

dhermes commented Mar 18, 2016

Looks fine (at least no issues with the code, though I realize this is a "design"-off challenge)

@tseaver
Copy link
Contributor Author

tseaver commented Mar 18, 2016

There are separate flags for "Leave Comments" and "Use Status API":

screenshot from 2016-03-18 14 04 20

@theacodes
Copy link
Contributor

FWIW, codecov can be bit better than coveralls at times.

On Fri, Mar 18, 2016 at 11:06 AM Tres Seaver [email protected]
wrote:

There are separate flags for "Leave Comments" and "Use Status API":

[image: screenshot from 2016-03-18 14 04 20]
https://cloud.githubusercontent.com/assets/242750/13887533/882a7bae-ed12-11e5-9d6c-e9b8c880f16f.png

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#1637 (comment)

@dhermes
Copy link
Contributor

dhermes commented Mar 18, 2016

We don't have any allegiance, just in the "ain't broke" state

jgeewax
jgeewax reviewed Mar 20, 2016
View reviewed changes
docs/pubsub-usage.rst
... batch.publish('this is the first message_payload')
... batch.publish('this is the second message_payload',
... attr1='value1', attr2='value2')
>>> from gcloud.pubsub.subscription import AutoAck

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver tseaver mentioned this pull request May 12, 2016
Closed
@tseaver
Copy link
Contributor Author

tseaver commented Jul 11, 2016

In favor of #1636

@tseaver tseaver closed this Jul 11, 2016
@tseaver tseaver deleted the 798-jgeewax_strategy branch July 11, 2016 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tseaver @coveralls @dhermes @theacodes @jgeewax @googlebot