Update samples to support latest Google Cloud Python#656
Merged
Conversation
added 9 commits
November 15, 2016 10:17
Change-Id: I11180caa0f97743fc3475e05fc44d5686702aabd
Change-Id: I78ceda896df08c433620b3a40e21fef7bb72f4a2
Change-Id: I07d16616e373d925f6ce5e9e47ab2fb7d073eb59
Change-Id: I6cbcca11be84baa7c40eed14b342cc916b9116d9
Change-Id: Ibd8cc3b651e666c5c5e7d9537a690cf4e9acc03a
Change-Id: I3ea731e4022610802b262a93197dee17b2e09838
Change-Id: Iac57ac0708fadc11f358c1599f4d1ce94b0dac8b
Change-Id: Ic9731e804792aeaeee51ca87ab7dc5971fe7ef5b
googlebot
added
the
cla: yes
jerjou
approved these changes
Nov 15, 2016
|
|
||
| return tasks, cursor | ||
| tasks = list(page) | ||
| next_cusor = query_iter.next_page_token |
| requirements = [ | ||
| requirement for requirement in requirements if not | ||
| is_ignored(requirement)] | ||
|
|
Contributor
There was a problem hiding this comment.
Could put this conditional in the previous comprehension, no?
| """Ignores certain libraries.""" | ||
| for library in IGNORED_LIBRARIES: | ||
| if requirement.startswith(library): | ||
| return True |
Contributor
There was a problem hiding this comment.
Could do this more succinctly:
return any(requirement.startswith(l) for l in IGNORED_LIBRARIES)
Contributor
Author
There was a problem hiding this comment.
I like it, but it's almost too clever. I'm going to leave as-is for now.
Contributor
There was a problem hiding this comment.
Don't care either way but don't think it's too clever, a bit more English-esque if anything.
Contributor
|
LGTM, shoutout to @dhermes for a lot of nice refactors that make these samples look a lot nicer. |
Contributor
Author
|
@dpebot merge when travis passes, please. |
Collaborator
|
Okay! I'll merge when all statuses are green. |
dpebot
added
the
automerge
Change-Id: Iab5bd07b405cf57fa7407efcfa188d1320e5f867
dhermes
reviewed
Nov 15, 2016
| # Load at most 25 results. You can change the max_results argument to load | ||
| # more rows from BigQuery, but note that this can take some time. It's | ||
| # preferred to use a query. | ||
| rows = table.fetch_data(max_results=25) |
Contributor
There was a problem hiding this comment.
You may prefer to just consume the iterator right here:
rows = list(table.fetch_data(max_results=25))There is too much suspense between this and the for row in rows (when the request(s) actually get made)
Contributor
Author
There was a problem hiding this comment.
Fair enough, done.
theacodes
removed
the
automerge
added 2 commits
November 15, 2016 14:02
Change-Id: I33997d42c6202c7f916bcebd4c5a9499392ac4db
Change-Id: I9609daebbd2bfd62706193e80fde245bd1ae2566
theacodes
added
the
automerge
8 tasks
9 tasks
9 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(I recommend reviewing commit by commit)