Skip to content

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Sep 29, 2017

Note that both new tests are failing.

Because they have to be explicitly enabled (via the `GOOGLE_CLOUD_TESTS_USER_PROJECT`` environment variable, which we aren't yet setting for CI), this PR is OK to merge.

@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. testing labels Sep 29, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 29, 2017
@tseaver tseaver requested a review from dhermes September 29, 2017 21:27
Copy link
Contributor

@dhermes dhermes left a comment

Choose a reason for hiding this comment

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

LGTM, still wish you'd try to factor the try/finally out of these test cases.

dest = with_user_project.blob('dest', encryption_key=KEY)
token, rewritten, total = dest.rewrite(source)

self.assertEqual(token, None)

This comment was marked as spam.

@tseaver
Copy link
Contributor Author

tseaver commented Sep 29, 2017

@frankyn notes offline:

Rewrite has a known issue with userproject and the Storage team has a fix for it, but pending review and release. I'll keep you posted.

@tseaver tseaver force-pushed the storage-requester_pays-feature branch from 1c67795 to cd6d50a Compare October 2, 2017 22:45
Note that both new tests are failing when run with the
GOOGLE_CLOUD_TESTS_USER_PROJECT variable set, due to a known back-end
incompatibility between 'objects.rewrite' and 'userProject' (fix pending
review / release).
@tseaver tseaver changed the title WIP: Test 'Blob.rewrite' w/ 'user_project' set. Test 'Blob.rewrite' w/ 'user_project' set. Oct 5, 2017
@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 tseaver merged commit d34dfe5 into googleapis:storage-requester_pays-feature Oct 5, 2017
@tseaver tseaver deleted the storage-requester_pays-systests-9 branch October 5, 2017 16:18
tseaver added a commit that referenced this pull request Oct 5, 2017
Test 'Blob.rewrite' w/ 'user_project' set.
tseaver added a commit that referenced this pull request Oct 10, 2017
Test 'Blob.rewrite' w/ 'user_project' set.
tseaver added a commit that referenced this pull request Oct 10, 2017
Test 'Blob.rewrite' w/ 'user_project' set.
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. backend cla: yes This human has signed the Contributor License Agreement. testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants