Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Storage: Add benchwrapper, support STORAGE_EMULATOR_HOST. #9219

Merged
merged 5 commits into from
Sep 16, 2019
Merged

Storage: Add benchwrapper, support STORAGE_EMULATOR_HOST. #9219

merged 5 commits into from
Sep 16, 2019

Conversation

jeanbza
Copy link
Contributor

@jeanbza jeanbza commented Sep 12, 2019

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 12, 2019
@tseaver tseaver changed the title storage: add benchwrapper, support STORAGE_EMULATOR_HOST Storage: Add benchwrapper, support STORAGE_EMULATOR_HOST. Sep 12, 2019
@tseaver tseaver added the api: storage Issues related to the Cloud Storage API. label Sep 12, 2019
@busunkim96
Copy link
Contributor

@jadekler Would you mind running nox -s blacken to appease the linter? That will run a code formatter.

@jeanbza
Copy link
Contributor Author

jeanbza commented Sep 12, 2019

@jadekler Would you mind running nox -s blacken to appease the linter? That will run a code formatter.

Done!

tseaver
tseaver suggested changes Sep 12, 2019
View reviewed changes
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

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

In addition to in-line review comments, please blacken the code (nox -e blacken).

core/google/cloud/environment_vars.py Outdated Show resolved Hide resolved
storage/google/cloud/storage/blob.py Outdated Show resolved Hide resolved
storage/google/cloud/storage/blob.py Show resolved Hide resolved
storage/google/cloud/storage/client.py Show resolved Hide resolved
storage/test_utils/storage.proto Show resolved Hide resolved
storage/test_utils/benchwrapper.py Outdated Show resolved Hide resolved
storage/test_utils/benchwrapper.py Show resolved Hide resolved
@jeanbza
Copy link
Contributor Author

jeanbza commented Sep 12, 2019

nox -e blacken

I did that already - is there more I need to do? :)

deklerk at deklerk-macbookpro2 in ~/workspace/google-cloud-python/storage on storage_emulator
$ nox -e blacken
nox > Running session blacken
nox > Creating virtual environment (virtualenv) using python3.6 in .nox/blacken
nox > pip install black==19.3b0
nox > black docs google tests noxfile.py setup.py
All done! ✨ 🍰 ✨
31 files left unchanged.
nox > Session blacken was successful.
deklerk at deklerk-macbookpro2 in ~/workspace/google-cloud-python/storage on storage_emulator
$

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Sep 13, 2019
@tseaver
Copy link
Contributor

tseaver commented Sep 13, 2019

@jadekler I've pushed the DRY changes to your branch.

tseaver
tseaver approved these changes Sep 13, 2019
View reviewed changes
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

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

LGTM pending CI green. @jadekler you will need to tweak the CLA stuff.

@busunkim96 busunkim96 added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Sep 13, 2019
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@jeanbza
Copy link
Contributor Author

jeanbza commented Sep 13, 2019

@googlebot I consent

@jeanbza
Copy link
Contributor Author

jeanbza commented Sep 13, 2019

@jadekler I've pushed the DRY changes to your branch.

Thank you!!

@jeanbza
Copy link
Contributor Author

jeanbza commented Sep 14, 2019

(could somebody merge? I don't have powers to :) )

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Sep 14, 2019
@jeanbza
Copy link
Contributor Author

jeanbza commented Sep 14, 2019

@googlebot I consent.

1 similar comment
@tseaver
Copy link
Contributor

tseaver commented Sep 16, 2019

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Sep 16, 2019
@tseaver tseaver merged commit 795bb32 into googleapis:master Sep 16, 2019
emar-kar pushed a commit to MaxxleLLC/google-cloud-python that referenced this pull request Sep 18, 2019
@jeanbza @emar-kar
Add support for 'STORAGE_EMULATOR_HOST' env var; add 'benchwrapper' s…
6c753d9 
…cript. (googleapis#9219)
emar-kar pushed a commit to MaxxleLLC/google-cloud-python that referenced this pull request Sep 18, 2019
@jeanbza @emar-kar
Add support for 'STORAGE_EMULATOR_HOST' env var; add 'benchwrapper' s…
8f795b0 
…cript. (googleapis#9219)
@simonvanderveldt simonvanderveldt mentioned this pull request Jan 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tseaver tseaver tseaver approved these changes

@busunkim96 busunkim96 Awaiting requested review from busunkim96

@crwilcox crwilcox Awaiting requested review from crwilcox

@frankyn frankyn Awaiting requested review from frankyn

Assignees
No one assigned
Labels
api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement. testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jeanbza @busunkim96 @googlebot @tseaver