Skip to content

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented May 5, 2021

Use an explicit helper client method for GET requests, rather than manipulating client's private _connection.api_request. As a benefit, tests get way clearer.

Toward #38

Based on top of the branch from #430. I will rebase when that PR merges.

@tseaver tseaver requested review from a team, andrewsg, busunkim96 and crwilcox May 5, 2021 21:23
@tseaver tseaver requested a review from a team as a code owner May 5, 2021 21:23
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label May 5, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 5, 2021
@tseaver tseaver marked this pull request as draft May 5, 2021 21:27
@tseaver tseaver force-pushed the 38-refactor-api-methods-_helpers branch 2 times, most recently from aab3324 to 4f1110c Compare May 6, 2021 17:10
@tseaver tseaver marked this pull request as ready for review May 6, 2021 17:50
@tseaver tseaver force-pushed the 38-refactor-api-methods-_helpers branch from 4bc1476 to 99e7e37 Compare May 6, 2021 18:04
@tseaver tseaver marked this pull request as draft May 6, 2021 20:01
@tseaver tseaver force-pushed the 38-refactor-api-methods-_helpers branch from 3942d75 to 2103462 Compare May 6, 2021 21:01
@tseaver tseaver marked this pull request as ready for review May 6, 2021 21:03
@tseaver tseaver force-pushed the 38-refactor-api-methods-_helpers branch from 2103462 to 794340a Compare May 7, 2021 17:50
@tseaver tseaver changed the title refactor: add / use 'Client._get_path' method refactor: add / use 'Client._get_resource' method May 7, 2021
@tseaver tseaver force-pushed the 38-refactor-api-methods-_helpers branch from 794340a to 9bae346 Compare May 7, 2021 17:54
@tseaver
Copy link
Contributor Author

tseaver commented May 7, 2021

Flaky systest reported in #435. Rerunning tests.

@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2021
@tseaver tseaver force-pushed the 38-refactor-api-methods-_helpers branch from 9bae346 to 44eeeb4 Compare May 10, 2021 15:24
@tseaver tseaver requested review from cojenco, frankyn and tritone and removed request for crwilcox May 20, 2021 14:46
tseaver added 2 commits May 20, 2021 11:34
For use by API-invoking methods using the 'GET' method.

Toward #38.
@tseaver tseaver force-pushed the 38-refactor-api-methods-_helpers branch from 385f51d to 392343d Compare May 20, 2021 15:34
method="GET",
path="%s/iam" % (self.path,),
info = client._get_resource(
"%s/iam" % (self.path,),

Choose a reason for hiding this comment

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

This is more of just a style thing, but in modern Python, should we be preferring str.format or f strings if Python 2 support has been dropped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once we drop Python2, I would definitely prefer f-strings.

Copy link

@chrisrossi chrisrossi left a comment

Choose a reason for hiding this comment

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

LGTM!

@tseaver tseaver added automerge Merge the pull request once unit tests and other checks pass. and removed automerge Merge the pull request once unit tests and other checks pass. labels Jun 7, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit e0f1b71 into master Jun 7, 2021
@gcf-merge-on-green gcf-merge-on-green bot deleted the 38-refactor-api-methods-_helpers branch June 7, 2021 14:32
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
Use an explicit helper client method for `GET` requests, rather than manipulating client's private `_connection.api_request`.  As a benefit, tests get *way* clearer.

Toward googleapis#38

~~Based on top of the branch from googleapis#430.  I will rebase when that PR merges.~~
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
Use an explicit helper client method for `GET` requests, rather than manipulating client's private `_connection.api_request`.  As a benefit, tests get *way* clearer.

Toward googleapis#38

~~Based on top of the branch from googleapis#430.  I will rebase when that PR merges.~~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the googleapis/python-storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants