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

feat: add ifgenerationMatch support, pt1 #123

Merged

frankyn merged 21 commits into googleapis:master from MaxxleLLC:metageneration_match_pt1

May 15, 2020

IlyaFaer commented May 1, 2020 •

edited

Loading

Add if_generation_match, if_generation_not_match, if_metageneration_match and if_metageneration_not_match arguments into methods:

Client(): lookup_bucket(), get_bucket()

Bucket(): get_blob(), delete_blob(), delete(), exists(), patch(), reload(), update()

Blob(): delete(), download_to_file(), download_to_filename(), download_as_string(), rewrite(), update_storage_class(), exists(), patch(), reload(), update(), reload()

Towards #127


          feat: add ifMetageneration*Match support, pt1

3fa6245

IlyaFaer added api: storage type: feature request labels

googlebot added the cla: yes label

IlyaFaer commented

View reviewed changes

google/cloud/storage/_helpers.py Show resolved Hide resolved


          fix unit tests, add test for helper

4cbad43

IlyaFaer commented

View reviewed changes

tests/unit/test_client.py Show resolved Hide resolved

IlyaFaer commented

View reviewed changes

google/cloud/storage/_helpers.py Show resolved Hide resolved


          fix unit tests

b905487

IlyaFaer requested a review from frankyn

May 1, 2020 10:36

IlyaFaer marked this pull request as ready for review

May 1, 2020 10:36


          add generation match args into more methods

ddecf53

IlyaFaer changed the title ~~feat: add ifMetageneration*Match support, pt1~~ feat: add if*generation*Match support, pt1

IlyaFaer added 2 commits

May 4, 2020 13:33


          feat: add if*generation*Match support, pt2

74961fa


          Lint fix.

7e08361

IlyaFaer mentioned this pull request

feat: add if*generation*Match support, pt2 #126

Closed

IlyaFaer requested a review from crwilcox

May 4, 2020 11:13

frankyn suggested changes

View reviewed changes

Contributor

frankyn left a comment

Client-side checks for if_*generation checks should not be added for this case.

Please remove use of _raise_for_more_than_one_none and let the service handle those errors.

google/cloud/storage/_helpers.py Outdated Show resolved Hide resolved

frankyn mentioned this pull request

Support If*generation preconditions #127

Closed

IlyaFaer added 4 commits

May 5, 2020 10:34


          delete "more than one set "checks

6f9f910


          del excess import

ff26d2f


          Merge branch 'metageneration_match_pt1' into metageneration_match_pt2

551df43


          delete "more than one set" checks

1f4d9c1

IlyaFaer mentioned this pull request

feat: add if*generation*match args into Bucket.delete_blobs() #130

Merged


          Merge branch 'master' into metageneration_match_pt1

0a34086

andrewsg requested changes

View reviewed changes

Contributor

andrewsg left a comment

Looks like the system tests are in part 2. So that we don't merge an incomplete change, please go ahead and merge part 2 into this PR and close that one. It will be a large PR but that's better than risking merging one and having the other delayed.

google/cloud/storage/_helpers.py Show resolved Hide resolved

google/cloud/storage/_helpers.py Show resolved Hide resolved

IlyaFaer added 4 commits

May 12, 2020 09:55


          Merge branch 'master' into metageneration_match_pt1

e1b67b5


          rename the helper; add error raising in case of wront parameters type

bd5b54f


          Merge branch 'metageneration_match_pt2' into metageneration_match_pt1

ea37318


          Merge branch 'metageneration_match_pt1' of https://github.com/q-logic…

9d19e15

…/python-storage into metageneration_match_pt1

IlyaFaer commented

View reviewed changes

Author

IlyaFaer left a comment •

edited

Loading

In fact, parts 1 and 2 are completely separated from each other, I've just tried to make PRs easier to read. Anyway after erasing the checks changes become much more shorter, so I suppose we can merge part 2 into this one.

google/cloud/storage/_helpers.py Show resolved Hide resolved

google/cloud/storage/_helpers.py Show resolved Hide resolved

Author

IlyaFaer commented May 12, 2020 •

edited

Loading

Lint session failed, but the problem is not related to this PR:

Running session lint
Creating virtual environment (virtualenv) using python3.8 in .nox/lint
pip install flake8 black==19.10b0
black --check docs google tests noxfile.py setup.py
All done! ✨ 🍰 ✨
35 files would be left unchanged.
flake8 google tests
google/cloud/storage/_signing.py:156:13: F523 '...'.format(...) has unused arguments at position(s): 0
Command flake8 google tests failed with exit code 1
Session lint failed.

IlyaFaer added the kokoro:force-run label

yoshi-kokoro removed the kokoro:force-run label


          Merge branch 'master' into metageneration_match_pt1

d8f790a

andrewsg requested changes

View reviewed changes

Contributor

andrewsg left a comment

Thanks Ilya. This looks great, only thing to note is that it looks like the metageneration match could use a system test. I don't think there's utility in making a system test for each modified function, but adding a system test for the metageneration match in addition to the generation might be a sweet spot for effort/reward. Should be good to go after that's in.

IlyaFaer added 4 commits

May 14, 2020 14:21


          add more system tests

bbebfc1


          system tests fixes

e9748bb


          cleanup system test

3e83091


          fix comments

0dbd9a9

Author

IlyaFaer commented May 14, 2020

I've added more system tests, and re-reviewed it all once again - fixed a couple of nits.

IlyaFaer mentioned this pull request

feat: add *generation*match args into Blob.compose() #122

Merged

frankyn suggested changes

View reviewed changes

Contributor

frankyn left a comment

One nit, but otherwise LGTM. Pending @andrewsg's comments.

google/cloud/storage/blob.py Outdated Show resolved Hide resolved


          delete excess checks

dc0bb4f

frankyn approved these changes

View reviewed changes

Contributor

frankyn left a comment

LGTM, pending @andrewsg's approval.

andrewsg approved these changes

View reviewed changes

Contributor

andrewsg left a comment

Thanks!

frankyn merged commit 0944442 into googleapis:master

IlyaFaer deleted the metageneration_match_pt1 branch

May 15, 2020 07:49

IlyaFaer pushed a commit to MaxxleLLC/python-storage that referenced this pull request


          feat: add if*generation*Match support, pt1 (googleapis#123)

648d3c8

* feat: add ifMetageneration*Match support, pt1

* fix unit tests, add test for helper

* fix unit tests

* add generation match args into more methods

* feat: add if*generation*Match support, pt2

* Lint fix.

* delete "more than one set "checks

* del excess import

* delete "more than one set" checks

* rename the helper; add error raising in case of wront parameters type

* add more system tests

* system tests fixes

* cleanup system test

* fix comments

* delete excess checks

Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>

cojenco pushed a commit to cojenco/python-storage that referenced this pull request


          feat: add if*generation*Match support, pt1 (googleapis#123)

* feat: add ifMetageneration*Match support, pt1

* fix unit tests, add test for helper

* fix unit tests

* add generation match args into more methods

* feat: add if*generation*Match support, pt2

* Lint fix.

* delete "more than one set "checks

* del excess import

* delete "more than one set" checks

* rename the helper; add error raising in case of wront parameters type

* add more system tests

* system tests fixes

* cleanup system test

* fix comments

* delete excess checks

Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>

cojenco pushed a commit to cojenco/python-storage that referenced this pull request


          feat: add if*generation*Match support, pt1 (googleapis#123)

33622ae

* feat: add ifMetageneration*Match support, pt1

* fix unit tests, add test for helper

* fix unit tests

* add generation match args into more methods

* feat: add if*generation*Match support, pt2

* Lint fix.

* delete "more than one set "checks

* del excess import

* delete "more than one set" checks

* rename the helper; add error raising in case of wront parameters type

* add more system tests

* system tests fixes

* cleanup system test

* fix comments

* delete excess checks

Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>

release-please bot mentioned this pull request

chore(main): release 3.0.0 #688

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage cla: yes type: feature request