Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

refactor: add / use 'Client._get_resource' method - #431

Merged
gcf-merge-on-green[bot] merged 18 commits into
masterfrom
38-refactor-api-methods-_helpers
Jun 7, 2021
Merged

refactor: add / use 'Client._get_resource' method#431
gcf-merge-on-green[bot] merged 18 commits into
masterfrom
38-refactor-api-methods-_helpers

Conversation

@tseaver

@tseavertseaver commented May 5, 2021

Copy link
Copy Markdown
Contributor

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 crwilcoxMay 5, 2021 21:23
@product-auto-labelproduct-auto-labelBot added the api: storage Issues related to the googleapis/python-storage API. label May 5, 2021
@google-clagoogle-claBot 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
tseaverforce-pushed the 38-refactor-api-methods-_helpers branch 2 times, most recently from aab3324 to 4f1110cCompareMay 6, 2021 17:10
@tseaver
tseaver marked this pull request as ready for review May 6, 2021 17:50
@tseaver
tseaverforce-pushed the 38-refactor-api-methods-_helpers branch from 4bc1476 to 99e7e37CompareMay 6, 2021 18:04
@tseaver
tseaver marked this pull request as draft May 6, 2021 20:01
@tseaver
tseaverforce-pushed the 38-refactor-api-methods-_helpers branch from 3942d75 to 2103462CompareMay 6, 2021 21:01
@tseaver
tseaver marked this pull request as ready for review May 6, 2021 21:03
@tseaver
tseaverforce-pushed the 38-refactor-api-methods-_helpers branch from 2103462 to 794340aCompareMay 7, 2021 17:50
@tseavertseaver changed the title refactor: add / use 'Client._get_path' methodrefactor: add / use 'Client._get_resource' methodMay 7, 2021
@tseaver
tseaverforce-pushed the 38-refactor-api-methods-_helpers branch from 794340a to 9bae346CompareMay 7, 2021 17:54
@tseaver

Copy link
Copy Markdown
ContributorAuthor

Flaky systest reported in #435. Rerunning tests.

@tseavertseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2021
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2021
@tseaver
tseaverforce-pushed the 38-refactor-api-methods-_helpers branch from 9bae346 to 44eeeb4CompareMay 10, 2021 15:24
@tseaver
tseaver requested review from cojenco, frankyn and tritone and removed request for crwilcoxMay 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
tseaverforce-pushed the 38-refactor-api-methods-_helpers branch from 385f51d to 392343dCompareMay 20, 2021 15:34
method="GET",
path="%s/iam" % (self.path,),
info = client._get_resource(
"%s/iam" % (self.path,),

Copy link
Copy Markdown

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
Copy Markdown
ContributorAuthor

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.

@chrisrossichrisrossi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@tseavertseaver 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-greenBot merged commit e0f1b71 into masterJun 7, 2021
@gcf-merge-on-green
gcf-merge-on-greenBot 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 freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: storageIssues related to the googleapis/python-storage API.cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tseaver@chrisrossi@yoshi-kokoro