Abacn commented on code in PR #41252: URL: https://github.com/apache/beam/pull/41252#discussion_r4559222210
########## CHANGES.md: ########## @@ -198,6 +198,8 @@ ## Breaking Changes +* (Python) The default GCS staging bucket ownership check is now strict. If ownership of an existing default bucket cannot be verified (the bucket has no project number, or the project number cannot be resolved via the Cloud Resource Manager API), a `ValueError` is raised instead of logging a warning and skipping the check ([#X](https://github.com/apache/beam/issues/X)). Review Comment: We can use this PR number (to replace the placeholder) ########## sdks/python/apache_beam/io/gcp/gcsio_test.py: ########## @@ -448,9 +448,10 @@ def test_get_or_create_default_gcs_bucket_ownership_no_bucket_number( mock_gcsio.get_bucket.return_value = mock_bucket options = SampleOptions(DEFAULT_GCP_PROJECT, 'us-central1') - bucket = gcsio.get_or_create_default_gcs_bucket(options) + with self.assertRaises(ValueError) as ctx: Review Comment: Is it possible to preserve a test for the happy path (the check passing if the project number is expected) by mocking client? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
