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

Add --password flag for providing API password #74

Merged
merged 1 commit into from
Dec 15, 2018

Conversation

dotlambda
Copy link
Contributor

I was not sure if it was right thing to add HASSIO_PASSWORD .

Should I add tests for this functionality?

homeassistant_cli/cli.py Outdated Show resolved Hide resolved
homeassistant_cli/autocompletion.py Outdated Show resolved Hide resolved
@dotlambda dotlambda force-pushed the api-password branch 2 times, most recently from 24d7c16 to d113643 Compare December 13, 2018 14:57
@maxandersen
Copy link
Contributor

I only didn't add it since I was always using token - hoping someone else would do it if we needed it :)

If you fix the error/issues the hound found I'm fine merging this in, having a basic test for it will be awesome. You can look at extending test_defaults.py or add your own for it.

@@ -64,6 +64,8 @@ def restapi(

if ctx.token:
headers["Authorization"] = "Bearer {}".format(ctx.token)
if ctx.password is not None:
headers["x-ha-access"] = ctx.password
Copy link
Contributor

Choose a reason for hiding this comment

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

adding both headers will never make sense - but it also don't really hurt so let's just keep it simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there maybe some way to tell click to output an error if both, token and password, are provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems there is none but we can easily implement our own: pallets/click#257. Should I do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that would be great, but i'm ok if you do that as a separate pr since its mainly a nice to have.

@dotlambda dotlambda force-pushed the api-password branch 2 times, most recently from a4e9de3 to 0a9fa31 Compare December 14, 2018 19:32
@dotlambda
Copy link
Contributor Author

Added a test case for HASS_PASSWORD to test_defaults.py and removed the changes to helper.py.

@maxandersen
Copy link
Contributor

Tested this and works! Thanks @dotlambda

@maxandersen maxandersen merged commit c7c9017 into home-assistant-ecosystem:dev Dec 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants