Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage): allow non default service account #11137

Merged
merged 3 commits into from
Nov 15, 2024
Merged

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
9 checks passed
@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