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.

Sign up for GitHub

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

Jump to bottom

[gcr] eliminate GCR #12963

Merged
merged 4 commits into from
May 3, 2023
Merged

[gcr] eliminate GCR #12963

merged 4 commits into from
May 3, 2023

Conversation

danking
Copy link
Contributor

@danking danking commented May 2, 2023

No description provided.

@danking
Copy link
Contributor Author

danking commented May 2, 2023

The remaining occurrences are not relevant to production work:

/Users/dking/projects/hail/batch/batch/worker/worker.py:484:            # * gcr.io/hail-vdc/hailgenetics/python-dill
/Users/dking/projects/hail/batch/batch/worker/worker.py:705:        # DockerError(500, "Head https://gcr.io/v2/genomics-tools/samtools/manifests/latest: unknown: Project 'project:genomics-tools' not found or deleted.")
/Users/dking/projects/hail/datasets/extract/extract_CADD.py:26:    j.image("gcr.io/broad-ctsa/datasets:050521")
/Users/dking/projects/hail/datasets/extract/extract_1000_Genomes_NYGC_30x_GRCh38.py:12:    j.image("gcr.io/broad-ctsa/datasets:041421")
/Users/dking/projects/hail/datasets/extract/extract_1000_Genomes_NYGC_30x_GRCh38.py:19:    j.image("gcr.io/broad-ctsa/datasets:041421")
/Users/dking/projects/hail/datasets/extract/extract_1000_Genomes_NYGC_30x_GRCh38.py:26:    j.image("gcr.io/broad-ctsa/datasets:041421")
/Users/dking/projects/hail/hail/scripts/update-terra-image.py:33:Image URL: `us.gcr.io/broad-dsp-gcr-public/{image_name}:{image_version}`
/Users/dking/projects/hail/hail/python/test/hailtop/utils/test_utils.py:115:    x = parse_docker_image_reference('gcr.io/hail-vdc/batch-worker:123fds312')
/Users/dking/projects/hail/hail/python/test/hailtop/utils/test_utils.py:116:    assert x.domain == 'gcr.io'
/Users/dking/projects/hail/hail/python/test/hailtop/utils/test_utils.py:120:    assert x.name() == 'gcr.io/hail-vdc/batch-worker'
/Users/dking/projects/hail/hail/python/test/hailtop/utils/test_utils.py:121:    assert str(x) == 'gcr.io/hail-vdc/batch-worker:123fds312'
/Users/dking/projects/hail/hail/python/hail/docs/change_log.md:278:- (hail#12230) The python-dill Batch images in `gcr.io/hail-vdc` are no longer supported.
/Users/dking/projects/hail/hail/python/hailtop/utils/utils.py:707:        # DockerError(500, "Head https://gcr.io/v2/genomics-tools/samtools/manifests/latest: unknown: Project 'project:genomics-tools' not found or deleted.")
/Users/dking/projects/hail/hail/python/hailtop/utils/utils.py:1061:            return self.domain is not None and (self.domain == 'gcr.io' or self.domain.endswith('docker.pkg.dev'))
/Users/dking/projects/hail/hail/python/hailtop/aiocloud/aiogoogle/client/container_client.py:6:        super().__init__(f'https://gcr.io/v2/{project}', **kwargs)
/Users/dking/projects/hail/datasets/extract/extract_dbSNP.py:22:    j.image("gcr.io/broad-ctsa/datasets:050521")
/Users/dking/projects/hail/infra/gcp/main.tf:55:    "gcr.io/${var.gcp_project}"
/Users/dking/projects/hail/infra/gcp/main.tf:375:  display_name = "push to gcr.io"
/Users/dking/projects/hail/infra/gcp-broad/main.tf:57:    "gcr.io/${var.gcp_project}"
/Users/dking/projects/hail/infra/gcp-broad/main.tf:417:  display_name = "push to gcr.io"

daniel-goldstein
daniel-goldstein previously requested changes May 2, 2023
View reviewed changes
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.

Just a nit

infra/gcp/README.md Outdated
@@ -209,7 +209,7 @@ You can now install Hail:
the $HAIL/infra/gcp directory, unless otherwise stated.

- Run the following to authenticate docker and kubectl with the new
container registry and kubernetes cluster, respectively.
artifact registry and kubernetes cluster, respectively.

```
./bootstrap.sh configure_gcloud <ZONE>
Copy link
Contributor

Choose a reason for hiding this comment

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

These instructions were a little broken I guess if you were using AR but we should fix these to provide both AR region and k8s zone. Doesn't matter to me whether they're environment variables or inputs to the script

@danking
Copy link
Contributor Author

danking commented May 2, 2023

Alright, once this merges I will destroy the associated buckets:

gsutil rm -r gs://hail-vdc.appspot.com
gsutil rm -r gs://artifacts.hail-vdc.appspot.com
gsutil rm -r gs://staging.hail-vdc.appspot.com

And disable the Container Registry API and the App Engine API.

@danking danking merged commit 36fae4a into hail-is:main May 3, 2023
ehigham added a commit to ehigham/hail that referenced this pull request May 11, 2023
@ehigham
[benchmark] Make benchmarks run in Google Artifact Repository
3f07cb8 
Related: hail-is#12963
@ehigham ehigham mentioned this pull request May 11, 2023
Merged
danking pushed a commit that referenced this pull request May 11, 2023
@ehigham
[benchmark] Make benchmarks run in Google Artifact Repository (#13039)
d42d89e 
Related: #12963
Remember to add your service account to the new respository policy
bindings:
```
$ gcloud artifacts repositories add-iam-policy-binding 'hail-benchmarks' \
  --member='serviceAccount:YOUR_SERVICE_ACCOUNT' \
  --role='roles/artifactregistry.repoAdmin' \
  --location='us' \
  --project='broad-ctsa'
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniel-goldstein daniel-goldstein daniel-goldstein approved these changes

Assignees

@daniel-goldstein daniel-goldstein

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danking @daniel-goldstein