Skip to content

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Sep 25, 2017

Closes #3474.

Needs extra system tests which pass user_project to the various APIs which take it, in the context of a bucket which is created with requester_pays enabled.

@tseaver tseaver added api: storage Issues related to the Cloud Storage API. do not merge Indicates a pull request not ready for merge, due to either quality or timing. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Sep 25, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 25, 2017
@dhermes
Copy link
Contributor

dhermes commented Sep 25, 2017

@tseaver How much of this has been reviewed and how much is new? (Sorry if that is a question I should know the answer to.)

@tseaver
Copy link
Contributor Author

tseaver commented Sep 25, 2017

@dhermes This was all reviewed back in June: I just re-built the branch because the reversion made it hard to see what would be landed this time.

@dhermes
Copy link
Contributor

dhermes commented Sep 25, 2017

@tseaver Great. So no review needed?

@tseaver
Copy link
Contributor Author

tseaver commented Sep 25, 2017

@dhermes Not until we resolve whether / how we will implement the additional system tests.

@tseaver tseaver force-pushed the storage-requester_pays-feature branch 2 times, most recently from d34dfe5 to ec34758 Compare October 5, 2017 17:29
@tseaver
Copy link
Contributor Author

tseaver commented Oct 5, 2017

@lukesneeringer I believe that this branch is ready to merge to master. Two open issues:

@tseaver tseaver removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 5, 2017
@tseaver
Copy link
Contributor Author

tseaver commented Oct 5, 2017

Unrelated AppVeyor failure (see #4128).

@tseaver
Copy link
Contributor Author

tseaver commented Oct 9, 2017

@lukesneeringer AFAIK, this branch is ready to merge.

Also, add 'requester_pays' argument to 'Client.create_bucket'.

Add a system test which exercises the feature.

Note that the new system test is skipped, because 'Buckets.insert' fails
with the 'billing/requesterPays' field set, both in our system tests and
in the 'Try It!' form in the docs.

Toward #3474.
* Add abstract '_PropertyMixin.user_project' property.

* Support 'user_project' in '_PropertyMixin.{reload,patch}'.

* Add 'user_project' param to 'Bucket.__init__'.

* Save and expose via read-only 'user_project' property.

* Implement 'Blob.user_property' via bucket's value.
* Block 'Bucket.create' if 'user_project' set:  the API does not accept that parameter.
Back-end used to reject it, but now allows it.
…ct' set (#4084)

* Pass through extra posargs for system tests.

* Plumb 'user_project' arg through 'Client.bucket'.
@tseaver tseaver force-pushed the storage-requester_pays-feature branch from 41fca6f to b0e1c96 Compare October 10, 2017 21:19
@tseaver tseaver merged commit aeb4cd4 into master Oct 12, 2017
@tseaver tseaver deleted the storage-requester_pays-feature branch October 12, 2017 14:43
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 Cloud Storage API. cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants