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

feat: add username and password login #418

Merged
merged 6 commits into from
Jul 28, 2022
Merged

Conversation

jeffreyssmith2nd
Copy link
Contributor

@jeffreyssmith2nd jeffreyssmith2nd commented Jun 29, 2022

This PR adds the ability for a user to login to InfluxDB using a username and password rather than a token. A user can specify both the username and password to be saved and used for all requests. Or, a user can specify just the username and they will be prompted for the password each time.
Note: this feature is OSS only

@jeffreyssmith2nd jeffreyssmith2nd marked this pull request as ready for review July 1, 2022 13:07
Copy link
Contributor

@candrewlee14 candrewlee14 left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple questions.

})
res, err := client.SigninApi.PostSignin(ctx).ExecuteWithHttpInfo()
if err != nil {
return "", err
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe if an error happens here, you could again print to the user that this is only supported for OSS, in case that clarifies their problem. What does the error look like when you try to make this request to cloud?

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 simply 404s because cloud tries to do some redirect stuff that we don't follow. I can wrap this error with a suggestion to make sure its not running against cloud.

Copy link
Contributor

@candrewlee14 candrewlee14 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants