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

Consider dropping most environment variables #555

Closed
jamessynge opened this issue Aug 24, 2018 · 8 comments
Closed

Consider dropping most environment variables #555

jamessynge opened this issue Aug 24, 2018 · 8 comments

Comments

@jamessynge
Copy link
Contributor

We have PANDIR, POCS, etc. And then we have config files that have the values of those envvars. It seems like we should have as few envvars as possible, and infer them where possible, or record them in config files where not.

@wtgee and @AnthonyHorton, your thoughts please.

@wtgee
Copy link
Member

wtgee commented Aug 24, 2018

I'm not opposed to this, more just curious about which ones are annoying you? Mine are set once and then I never really think about them. Is the question prompted by work on the installation script or something else? Also, I know we had the conversation somewhere else earlier, can't seem to find.

I think we only really use PANDIR and POCS. Could derive POCS certainly although it is used a lot and I frequently cd $POCS (etc) as part of my workflow.

I've recently added a SOLVE_FIELD in some situations but we could get rid of that.

Probably don't need $PANLOG, $PAWS.

Pretty sure we don't use $PANUSER a lot but I'm a little skeptical to get rid of it.

@jamessynge
Copy link
Contributor Author

I was reviewing @AnthonyHorton's change which adds a pyro config file, and that includes a path starting with /var/pantoptes. It seems to me that we should try to avoid duplicating the paths.

So, I'd like to either extend our YAML support with the ability to evaluate env vars, or reduce the use of the env vars... or both if there are some that really aren't needed.

@jamessynge
Copy link
Contributor Author

Perhaps we should have a config file in PANDIR with the mapping from names to paths? Then those are available to all the compatible apps we write that we usually place in /var/panoptes/.

@wtgee
Copy link
Member

wtgee commented Aug 25, 2018

A mapping for directories specifically? Might be a good idea to pull that out of pocs.yaml and put directly in $PANDIR.

@AnthonyHorton
Copy link
Collaborator

I'm supportive of the idea of removing the dependence on environment variables, and instead putting the items we really need in the config files and inferring the rest. On general principles keeping all the important configuration in the conf_files directory, rather than having some of it in user's .bashrc, .profile, etc. seems like a good idea to me.

FWIW the setting of base: /var/panoptes in the pyro_camera.yaml file that I added isn't a new thing. The directories section of that file is just a copy of the directories section in pocs.yaml. We've had a situation for a long time were we have an environment variable for something (PANDIR) as well as a config entry for the same thing (config['directories']['base']), which is obviously a recipe for confusing errors if they ever have different values. We should definitely be setting these things in only one place, & my preference would be the config files.

Having $POCS, $PANDIR, etc. defined in your shell can be handy, but of course it will still be possible to do that even if POCS itself no longer uses the environment variables. We could make that easy by having a script that parses the config files and sets the corresponding environment variables for you, or adds them to your .profile.

In terms of what to include $PANDIR and $PANUSER (or their config file equivalents) seem to be the only essential ones.

@jamessynge
Copy link
Contributor Author

@AnthonyHorton I agree that the problem predated your change; my goal is simply to get agreement on the fact that there is an issue and then agreement on what we can do about it. And I quite like the idea of a script that can generate the envvar settings for you.

So, at the limit we could have no env vars required, with /var/panoptes being assumed if PANDIR is not specified. And if folks want POCS, astrometry, PIAA, etc. elsewhere, they can add symlinks into the PANDIR.

Can you remind me what we use PANUSER for?

@wtgee
Copy link
Member

wtgee commented Aug 27, 2018

Huntsman runs everything as a different user under /var/huntsman. Works great to just set PANUSER and PANDIR. I believe our travis testing also uses PANUSER.

I've started stripping a few env variables in the PRs I am working on.

I've added #564 with some other config related items I've been thinking about. Please add anything to that overview issue.

@wtgee
Copy link
Member

wtgee commented Feb 16, 2020

The docker branch (soon to be merged into develop via #951) assumes there is an environment file in $PANDIR/.env (can be read via dotenv). This environment file currently consists of the following:

PANDIR=/var/panoptes
POCS=/var/panoptes/POCS
PANLOG=/var/panoptes/logs

Additionally, for those running the docker services there should be a:

LOCAL_USER_ID=1000

(This may disappear in future but is necessary now. Also technically it should be the output from id -u not just 1000).

If accessing GCP services then there should also be a:

GOOGLE_APPLICATION_CREDENTIALS=/path/to/service-account-key.json

The .env file helps start up the config server and everything else should be queried via the config server and not rely on env vars.

This means that there must be a one-time manual configuration of the .env file (i.e. it can't be completely self-discoverable).

I'm not sure there is much utility in keeping this broad issue open. As part of my issue cleanup I'm closing this. Can be reopened if needed.

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

No branches or pull requests

3 participants