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

Experimental #139

Merged
merged 158 commits into from
Dec 20, 2020
Merged

Experimental #139

merged 158 commits into from
Dec 20, 2020

Conversation

Slyke
Copy link
Collaborator

@Slyke Slyke commented Oct 6, 2020

No description provided.

Slyke added 30 commits May 10, 2020 16:26
Added backup and store menu
Copy link

@Paraphraser Paraphraser left a comment

Choose a reason for hiding this comment

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

I don't see the point of the "random password" in this particular situation.

I keep worrying that I have not made it clear what the associated environment variable is for. The clue is in the environment variable's name. It starts with "INFLUX". It does not start with "INFLUXDB" and it's not just a cosmetic thing. It's a really important difference.

The INFLUX_USERNAME and INFLUX_PASSWORD are affordances for parameters on the "influx" CLI command. This is the thing where you do into the container and type "influx".

IF you run the "influx" command AND you create a database administrator (which can be any username or password you like) AND you turn on INFLUXDB_HTTP_AUTH_ENABLED THEN these two environment keys save you (the administrator) from typing your username and password on the "influx" command or having to execute an AUTH statement inside the CLI. That is all they do.

What worries me about creating random passwords in a situation like this is that it tends to signal things like "this protects your database" and "these are the credentials you will need to use in NodeRed or Grafana to write or read the database.

Those two environment keys don't do that.

In setting up the ENV file with lots of inline comments, I was responding to the various misunderstandings I've encountered both in my own journey and in answering questions in issues and on Discord. I'd really rather not unleash a new crop of misunderstandings if it can be avoided.

Having said all that, perhaps I'm worrying about not much. It just makes me a bit twitchy...

@Slyke
Copy link
Collaborator Author

Slyke commented Nov 14, 2020

Hey @Paraphraser I removed the password option (code is still there, just menu item is hidden) to avoid any confusion, and also updated the env vars in the service.yml for the ones you posted. I also added in links to documentation for services that have options and documentation on each of those services options pages.

Example:
image

I want to put the awesome documentation you wrote in the env file for https://github.com/SensorsIot/IOTstack/pull/148/files onto the InfluxDB document page, but didn't want to just copy and paste your work with the commit signed by me. Did you want to update that page?

I've also updated some of the rendering logic. It should be fixed, or at least warn you with a reason if there's issues rending the menus.
image

I could shrink the horizontal space required too, but it makes the rendering logic very complicated when wrapping lines, or limits the amount of characters per line. Currently it needs a 38h by 80v character terminal size.

You may also be interested in the Miscellaneous Commands > Install SSH keys from your Github (and the script it executes bash ./scripts/install_ssh_keys.sh) for rolling out deployments on new systems.

I'm ready to merge this now, unless you've found any other issues.

@Slyke Slyke merged commit ac2370e into master Dec 20, 2020
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.

3 participants