FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor(storage): deprecate Bucket.create() and requester_pays by IlyaFaer · Pull Request #9893 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

refactor(storage): deprecate Bucket.create() and requester_pays - #9893

Closed
IlyaFaer wants to merge 8 commits into
googleapis:masterfrom
MaxxleLLC:deprecate_create_bucket
ConversationCommits (8)ChecksFiles changed
Closed

refactor(storage): deprecate Bucket.create() and requester_pays#9893
IlyaFaer wants to merge 8 commits into
googleapis:masterfrom
MaxxleLLC:deprecate_create_bucket

Conversation

IlyaFaer commented Nov 26, 2019

Copy link
Copy Markdown

Towards #7762

Completing steps 1, 2 and 3 from the design doc:

IlyaFaer added the api: storage Issues related to the Cloud Storage API. label Nov 26, 2019
googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 26, 2019
IlyaFaer changed the title refactor(storage): deprecate Bucket.create() and requester_pays. refactor(storage): deprecate Bucket.create() and requester_pays Nov 26, 2019
IlyaFaer commented Nov 26, 2019
View reviewed changes
Comment thread storage/tests/unit/test_bucket.py
expected_cw = [((), expected_called_kwargs)]
self.assertEqual(_FakeConnection._called_with, expected_cw)

def test_create_w_user_project(self):

IlyaFaer Nov 26, 2019 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Most part of these tests had already been moved into client_test.py in this PR. Some will be moved with the current one

IlyaFaer commented Nov 26, 2019
View reviewed changes
Comment thread storage/tests/unit/test_client.py
http = _make_requests_session([_make_json_response(data)])
client._http_internal = http

bucket = client.create_bucket(bucket_name, requester_pays=True)

IlyaFaer Nov 26, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Arg is now deprecated

IlyaFaer commented Nov 26, 2019
View reviewed changes
Comment thread storage/google/cloud/storage/bucket.py
project=project,
location=location,
predefined_acl=predefined_acl,
predefined_default_object_acl=predefined_default_object_acl,

IlyaFaer Nov 26, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Deleting old implementation and mirroring to Client.create_bucket()

IlyaFaer requested review from crwilcox, frankyn and tseaver and removed request for crwilcox November 27, 2019 07:52
IlyaFaer marked this pull request as ready for review November 27, 2019 07:52

tswast commented Nov 27, 2019

Copy link
Copy Markdown
Contributor

Are there any code samples (here or in https://github.com/GoogleCloudPlatform/python-docs-samples/tree/master/storage) that still use these methods? If so, we should wait to deprecate until those are updated.

Also, we should make sure every API request you can make on Bucket can also be done from Client. It'll make for a much clearer developer story when Bucket can be treated as a dumb resource object.

IlyaFaer commented Nov 28, 2019

Copy link
Copy Markdown
Author

@tswast, yeah, I've checked python-docs-samples and didn't find any mentions of bucket.create() and requester_pays arg, only client.create_bucket().

Also, we should make sure every API request you can make on Bucket can also be done from Client. It'll make for a much clearer developer story when Bucket can be treated as a dumb resource object.

Well, I'm following the design doc for now, pushing piece by piece, as it's a lot of changes. If we need to mirror calls from Client() to Bucket(), maybe we should try to play around with getattr() and just make one redirecting function?

tswast commented Dec 17, 2019

Copy link
Copy Markdown
Contributor

maybe we should try to play around with getattr() and just make one redirecting function?

In my opinion, piece-by-piece as you're currently doing will make it easiest for others to contribute / onboard new teammembers.

IlyaFaer commented Jan 29, 2020

Copy link
Copy Markdown
Author

Friendly ping. This PR is waiting for review

plamut commented Feb 10, 2020

Copy link
Copy Markdown
Contributor

Please reopen in the new repo, thanks!

IlyaFaer commented Feb 10, 2020

Copy link
Copy Markdown
Author

Moved to googleapis/python-storage#49

IlyaFaer closed this Feb 10, 2020
IlyaFaer deleted the deprecate_create_bucket branch February 10, 2020 10:36
IlyaFaer mentioned this pull request Feb 10, 2020
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

frankyn Awaiting requested review from frankyn
crwilcox Awaiting requested review from crwilcox
tseaver Awaiting requested review from tseaver

Assignees

No one assigned

Labels

api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL