-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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(indexer): Load environment variables from .env #5944
Conversation
roninjin10
commented
Jun 9, 2023
•
edited
Loading
edited
- add dotenv to automatically load .env when running outside of docker
- This is mostly just a nice-to-have because these env variables are also used in docker-compose so it saves configuration
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
Semgrep found 1
Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information. Ignore this finding from use-tls. |
a228712
to
69a49cb
Compare
69a49cb
to
455b701
Compare
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 do we have a .env & also a toml configuration?
@hamdiallam you don't need a .env unless your toml file has environment variables in it. OUrs does for example here. This is also why we don't panic when no .env is found. It's perfectly normal to use the toml file without environment variables or to pass in environment variables in a way other than .env
|
We also need a .env file for docker-compose but that's unrelated to this pr |
455b701
to
4d62a59
Compare
Semgrep found 12
Please create a Linear ticket for this TODO. Ignore this finding from todos_require_linear. |
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
Hey @roninjin10! This PR has merge conflicts. Please fix them before continuing review. |
2af0314
to
538770d
Compare
Hey @roninjin10! This PR has merge conflicts. Please fix them before continuing review. |
fdfa71f
to
4717cc5
Compare
4717cc5
to
cd5def2
Compare
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'm not a fan of implicit configuration. It's too easy for users to accidentally leave a .env in their workspace and deploy that without easily knowing how the app behaves.
Though in our case, it seems fine since we use docker builds. But in general, I don't think this is good practice (including usage in the typescript BaseService apps)
cd5def2
to
0079df8
Compare
This PR has been added to the merge queue, and will be merged soon. |
This PR is next in line to be merged, and will be merged as soon as checks pass. |