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

[batch] use regional bucket for requester pays tests #12964

Merged
merged 2 commits into from
May 3, 2023

Conversation

danking
Copy link
Contributor

@danking danking commented May 2, 2023

I updated terraform but

  1. GCP Terraform state is still local on my laptop.

  2. GCP Terraform appears to not configure global-config. As such, I cannot thread the name of the bucket through to the tests the way we do with TEST_STORAGE_URI. For now, I've hardcoded the name (which is what we were doing previously). When we eventually get to testing recreation of GCP in a new project we'll have to address the global config then.

I updated terraform but

1. GCP Terraform state is still local on my laptop.

2. GCP Terraform appears to not configure global-config. As such, I cannot thread the name of the
   bucket through to the tests the way we do with TEST_STORAGE_URI. For now, I've hardcoded the
   name (which is what we were doing previously). When we eventually get to testing recreation of
   GCP in a new project we'll have to address the global config then.
@danking
Copy link
Contributor Author

danking commented May 2, 2023

Ah, I deleted the global-config as a part of switching to gcp-broad. Regardless, we'll need to address that as terraform is iteratively improved.

Copy link
Contributor

@daniel-goldstein daniel-goldstein left a comment

Choose a reason for hiding this comment

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

Ya agreed about the global-config. Since it's hardcoded right now it doesn't matter, but when we bring global-config ownership under terraform we will need to make sure that these resources get added to gcp terraform and subsequently threaded through to the global config.

@danking danking merged commit f81cec8 into hail-is:main May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants