search

simonsobs / sisock

Sisock ('saɪsɒk): streaming of Simons Obs. data over websockets for quicklook
Other
2 stars 0 forks source link

Use env var if they exist for base CONSTANTs #40

Closed BrianJKoopman closed 5 years ago

BrianJKoopman commented 5 years ago

This relates to #31. Even running on a second system with Docker, if not additional tools (like Docker swarm) are involved, then the name resolution we rely on within the Docker environment doesn't work across computers. We need a way to pass in our own addresses/etc. for the constants defined in base.py.

This simply tries to grab them from an environment variable, and if not available falls back to the previous settings. This is a nice solution for now since it doesn't require any changes in the data servers/sisock components to work.

If this looks alright, I'll add documentation to the various components for the new environment variables before merging.

ahincks commented 5 years ago

Looks good. Does this at the same time fix the problem with having password exposed in the repo that we discussed before? It is still there in the default: so I suppose it is the user's fault if good security is not implemented?

(I'm writing quickly as I'm dashing off and won't be able to look at this again for a couple of days, so do feel free to merge without my explicit approval if you're happy with the branch.)

BrianJKoopman commented 5 years ago

Does this at the same time fix the problem with having password exposed in the repo that we discussed before? It is still there in the default: so I suppose it is the user's fault if good security is not implemented?

Yeah, I thought I'd add the ability to put the user and secret in as environment variables as well. The idea being it'll still work for those who have the exposed secret in place, but they could update with new user/pass easily.

I'll add documentation for the new environment variables today and then merge. Thanks for taking a quick look!