Skip to content

Conversation

@frankyn
Copy link
Contributor

@frankyn frankyn commented Nov 14, 2024

Since Go Storage enables DirectPath by default, the client needs to enable the flag to allow non-default service accounts as well.

In talking with @mohanli-ml, there's an approval process that GCS has already done as a service and we only need to enable it client side. Mohan can i get your approval on this PR to ack this context?

Fixes: #11128
Triggered a continuous job with this PR:
image

@frankyn frankyn added the api: storage Issues related to the Cloud Storage API. label Nov 14, 2024
@frankyn frankyn changed the title test(storage): re-enable DetectDirectConnectivity integration test test(storage): enable DetectDirectConnectivity integration test Nov 14, 2024
@frankyn frankyn marked this pull request as ready for review November 14, 2024 23:52
@frankyn frankyn requested review from a team as code owners November 14, 2024 23:53
@frankyn frankyn changed the title test(storage): enable DetectDirectConnectivity integration test feat(storage): allow non default service account Nov 15, 2024
@frankyn frankyn requested a review from mohanli-ml November 15, 2024 01:36
Copy link
Contributor

@mohanli-ml mohanli-ml left a comment

Choose a reason for hiding this comment

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

Thanks Frank!

Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

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

We should also likely disable this test for cloudtop as well though since it seems to fail there...

@frankyn
Copy link
Contributor Author

frankyn commented Nov 15, 2024

Cloudtop works you just need to set environment variable:

GOOGLE_CLOUD_DISABLE_DIRECT_PATH=false

Direct Connectivity is disabled by default on cloudtop.

@frankyn frankyn merged commit 19f01c3 into main Nov 15, 2024
@frankyn frankyn deleted the fix-dp-check-config branch November 15, 2024 05:53
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

storage: TestIntegration_DetectDirectConnectivity failed

3 participants