-
Notifications
You must be signed in to change notification settings - Fork 234
The password provided to configure shouldn't be echo'd. #244
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
areese
commented
Jul 8, 2019
da10f3d to
a629bd5
Compare
|
This is very similar to: #123 |
| config = ProfileConfigProvider(profile).get_config() or DatabricksConfig.empty() | ||
| host = click.prompt(PROMPT_HOST, default=config.host, type=_DbfsHost()) | ||
| username = click.prompt(PROMPT_USERNAME, default=config.username) | ||
| password = 'stdin' |
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 my password is stdin? I know this password is probably not allowed in most sign up workflows in Databricks but I'd rather not special case this password..
Can we just add a flag called read_password_from_stdin or something?
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.
excellent point, let me rework this that way.
How about empty password? Would that be an ok choice or would you rather use a flag?
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.
I think a flag is probably just the cleanest way.
b723e68 to
57facd4
Compare
Add a --stdin option to configure, so that the password will always be read from stdin instead of stored in a file.
57facd4 to
44d3ba4
Compare
Codecov Report
@@ Coverage Diff @@
## master #244 +/- ##
==========================================
- Coverage 82.95% 82.63% -0.32%
==========================================
Files 31 31
Lines 2047 2062 +15
==========================================
+ Hits 1698 1704 +6
- Misses 349 358 +9
Continue to review full report at Codecov.
|
|
I'm going to close this. I ended up with a new patch to hide the token, and I never ended up using the stdin option after all. |