Visitar URL original
More gax paging fixes by tseaver · Pull Request #1870 · googleapis/google-cloud-python · GitHub
Skip to content

More gax paging fixes - #1870

Merged
tseaver merged 3 commits into
googleapis:masterfrom
tseaver:1855-moar_gax_paging_fixes
Jun 20, 2016
Merged

More gax paging fixes#1870
tseaver merged 3 commits into
googleapis:masterfrom
tseaver:1855-moar_gax_paging_fixes

Conversation

@tseaver

@tseaver tseaver commented Jun 20, 2016

Copy link
Copy Markdown
Contributor

Work toward fixing Pubsub system tests using GAX.

Correcting incomplete fixes in #1855.

Note that Topic.pull is still problematic (#1869).

@tseaver tseaver added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: pubsub Issues related to the Pub/Sub API. labels Jun 20, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 20, 2016
@daspecster daspecster mentioned this pull request Jun 20, 2016
Merged
7 tasks done
daspecster
daspecster reviewed Jun 20, 2016
View reviewed changes
Comment thread gcloud/pubsub/_gax.py Outdated
response = self._gax_api.publish(topic_path, message_pbs)
event = self._gax_api.publish(topic_path, message_pbs)
if not event.is_set():
import pdb; pdb.set_trace()

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver

tseaver commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

@daspecster I've fixed the lint issues with 2f347b8. The coverage gap was actually introduced in #1868: see #1871 for a fix.

@tseaver

tseaver commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

@daspecster I don't know why coveralls shows 100%. When I run tox -e cover locally, it shows a missing branch from line 206 -> exit of gcloud/test__helpers.py: that is what led me to #1871.

/me looks: I don't think coveralls is doing branch coverage. :(

@daspecster

daspecster commented Jun 20, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Looks like it doesn't lemurheavy/coveralls-public#31

@tseaver

tseaver commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

@daspecster I've rebased against master after merging #1871.

@daspecster

daspecster commented Jun 20, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Maybe this should be a separate issue, but perhaps we should have travis run the cover task separately with the coveralls task?

Made an issue for this: #1872

@daspecster

daspecster commented Jun 20, 2016

Copy link
Copy Markdown
Contributor

This LGTM @tseaver. Just to confirm, setting page_size = 0, the cloud side will just return whatever their max is for page_size? Or should it be None?

@tseaver

tseaver commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

@daspecster

Just to confirm, setting page_size = 0, the cloud side will just return whatever their max is for page_size? Or should it be None?

GAX-generated wrappers use 0 as the default for page_size.

@daspecster

daspecster commented Jun 20, 2016

Copy link
Copy Markdown
Contributor

Ok, let's merge!

@tseaver

tseaver commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

BTW, Travis used to run cover: we dropped it to allow the initial run to complete more quickly (a false economy, maybe).

@tseaver
tseaver merged commit 8c609e9 into googleapis:master Jun 20, 2016
@tseaver
tseaver deleted the 1855-moar_gax_paging_fixes branch June 20, 2016 18:56
This was referenced Jun 20, 2016
Closed
Closed
parthea pushed a commit that referenced this pull request Apr 1, 2026
@TrevorBergeron
fix: Fix bug with DataFrame.agg for string values (#1870)
80975b9
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. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tseaver @daspecster @googlebot