-
Couldn't load subscription status.
- Fork 27
Add Basic Authentication documentation and enhance InfluxDB migration auth guide #255
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
...uxDB migration guide
CLA assistant check
All committers have signed the CLA.
Hi, @javier, Please Check and Merge My Commit.
Thanks, @KanavCode . I would need the contributor agreement signed. Otherwise the automation doesn't allow me to preview and approve
Thanks, @KanavCode . I would need the contributor agreement signed. Otherwise the automation doesn't allow me to preview and approve
Yeah, Please Do.
Thanks, @KanavCode . I would need the contributor agreement signed. Otherwise the automation doesn't allow me to preview and approve
Yeah, Please Do.
Did you sign the CLA? The CLAAssistant comment above failed to verify it
Thanks, @KanavCode . I would need the contributor agreement signed. Otherwise the automation doesn't allow me to preview and approve
Yeah, Please Do.
Did you sign the CLA? The CLAAssistant comment above failed to verify it
Yupp, Done 👍
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.
Thanks! A nice contribution, but I am afraid it needs some work before we can publish it
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 would refrain from recommending this for production. Basic auth sends everything unencrypted, so it is very unsafe unless you have a proxy with TLS, which would be out of the scope of this guide
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 would move the note you have below about enterprise to this point instead
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.
This is misleading, as it allows just for one admin user and a readonly user. I think it would be best to say "Both the built-in admin and the optional read-only user can be configured for Basic Auth"
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 would move this note at the intro, before the Overview, so Enterprise users can skip the page completely
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 wouldn't feature the max response rows parameter here, as it has nothing to do with Basic Auth
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.
Where did you see this? It is not part of the questdb config.
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.
This example doesn't really says much, as client applications are not mentioned, and also coordinating application deployment with server restart can be tricky. I would rather leave the whole example out. You already mentioned how to configure and how to test earlier in the same page
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 would delete everything from this point on. Some things are unrelated, and some things are enterprise-only, which we already mentioned at the initial note.
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.
Why are you removing the enterprose option for user and password? Even if token is recommended, user and password for enterprise work as well
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 would remove this section. Not sure it is needed, and it is misleading as these variables are only for SERVER SIDE, not for client
Or individual variables
export QDB_HTTP_USER="admin"
export QDB_HTTP_PASSWORD="quest"
and the QDB_URL variable is not used by either client or server
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 would remove this section. Some things are enterprise only, and the rest are very mild generic recommendations
Changes Made
New Documentation
operations/basic-auth.md)Enhanced Documentation
guides/influxdb-migration.md)