FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

More gax paging fixes by tseaver · Pull Request #1870 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

More gax paging fixes - #1870

Merged
tseaver merged 3 commits into
googleapis:masterfrom
tseaver:1855-moar_gax_paging_fixes
Jun 20, 2016
ConversationCommits (3)ChecksFiles changed
Merged

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

Conversation

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 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 added the cla: yes This human has signed the Contributor License Agreement. label Jun 20, 2016
daspecster mentioned this pull request Jun 20, 2016
Merged
7 tasks
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 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 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 commented Jun 20, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

tseaver commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

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

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 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 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 commented Jun 20, 2016

Copy link
Copy Markdown
Contributor

Ok, let's merge!

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 merged commit 8c609e9 into googleapis:master Jun 20, 2016
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
fix: Fix bug with DataFrame.agg for string values (#1870)
80975b9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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

Back | FazBrowse Home | New Git URL