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

[ray launcher] fix key pair name - add user #1309

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

jieru-hu
Copy link
Contributor

Motivation

Right now the default key-pair name is hydra. However this won't work well if multiple users share one aws account - they won't be able to share the key pair with same names in the same account (easily.)

Have you read the Contributing Guidelines on pull requests?

Yes/No

Test Plan

Verified locally the variable was interpreted correctly and example applications run.

Related Issues and PRs

(Is this PR part of a group of changes? Link the other relevant PRs and Issues here. Use https://help.github.com/en/articles/closing-issues-using-keywords for help on GitHub syntax)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 19, 2021
@jieru-hu jieru-hu changed the title fix key pair name - add user [ray launcher] fix key pair name - add user Jan 19, 2021
@@ -58,7 +58,9 @@ class RayProviderConf:
region: str = "us-west-2"
availability_zone: str = "us-west-2a,us-west-2b"
cache_stopped_nodes: bool = False
key_pair: Dict[str, str] = field(default_factory=lambda: {"key_name": "hydra"})
key_pair: Dict[str, str] = field(
default_factory=lambda: {"key_name": "hydra-${env:USER,me}"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is "me" a good default value for a key name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably not a good name :) how about just user?

Copy link
Collaborator

Choose a reason for hiding this comment

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

why not hydra?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right now the key name is hydra-{$USER}, are you saying is $USER does not exist we do hydra-hydra?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to name it hydra-${env:USER, user} then :)

@jieru-hu jieru-hu merged commit 0af5d5c into facebookresearch:master Jan 20, 2021
@jieru-hu jieru-hu deleted the keypair branch January 20, 2021 18:31
jieru-hu added a commit to jieru-hu/hydra that referenced this pull request Jan 23, 2021
jieru-hu added a commit to jieru-hu/hydra that referenced this pull request Jan 27, 2021
jieru-hu added a commit that referenced this pull request Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants