-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-10532][EC2]Added --profile option to specify the name of profile #8696
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
Conversation
|
@teramonagi Could you clarify what these profiles are meant to be ? Are they IAM profiles or something else ? cc @nchammas |
|
@shivaram You can keep multiple sets of credentials in the same credentials files using different profile names. (Search the word "--profile" on the abobe link) |
|
+1 using AWS profile is considered to be best practice: https://blogs.aws.amazon.com/security/post/Tx3D6U6WSFGOK2H/A-New-and-Standardized-Way-to-Manage-Credentials-in-the-AWS-SDKs |
ec2/spark_ec2.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you expand this to explain what type of profile this is talking about? Like, maybe clarify whether it's an IAM or boto profile or whatever? As it stands now, I worry that this description isn't clear to people unfamiliar with this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually speaking, we can use any types of profile(AWS/boto). The remaining problem is just the loading order of these "profiles" as Jan Vlcinsky explained. I think that it is little bit long to explain that in commandline help.
I adde little modification(aws or boto config) in that help.
ec2/spark_ec2.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if I wanted to have a profile that's named "default"? I think it would better to have None as the default and to replace this with an is not None check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I will fix this quickly
|
This seems reasonable to me. Might be nice to have @nchammas take a quick peek. If I don't hear otherwise, I'll merge this by Monday. |
|
LGTM. I would update the PR description though so when this gets merged in the commit message has the rationale behind this change. |
|
Jenkins, this is ok to test. |
|
I'm going to run the Python style checker, then will merge this and update the PR description myself. Thanks @teramonagi. |
|
Jenkins, retest this please. |
|
Test build #44004 has finished for PR 8696 at commit
|
|
@JihongMA Thanks a lot Josh!!! |
No description provided.