Skip to content

allow specifying a VPC from another GCP project #5143

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

Merged
merged 4 commits into from
Apr 8, 2025

Conversation

cg505
Copy link
Collaborator

@cg505 cg505 commented Apr 8, 2025

Fixes #5141.

Tested (run the relevant ones):

  • Code formatting: install pre-commit (auto-check on commit) or bash format.sh
  • new unit test for config validation

@cg505 cg505 requested a review from Michaelvll April 8, 2025 01:51
@SeungjinYang SeungjinYang changed the title allow specicifying a VPC from another GCP project allow specifying a VPC from another GCP project Apr 8, 2025
Copy link
Collaborator

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

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

Thanks @cg505! LGTM.

Comment on lines +578 to +581
if len(split_vpc_value) != 2:
raise ValueError(f'Invalid VPC name: {specific_vpc_to_use}. '
'Please specify the VPC name in the format '
'PROJECT_ID/VPC_NAME.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add this check in jsonschema for config to fail early

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, might be good to add a quick unit test for specifying wrong format of vpc name.

@cg505
Copy link
Collaborator Author

cg505 commented Apr 8, 2025

test failure is unrelated.

@cg505 cg505 merged commit f520059 into skypilot-org:master Apr 8, 2025
35 of 37 checks passed
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.

[GCP] Allow use of a VPC from another GCP project
2 participants