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

Read .env for s3 credentials #4

Closed
wants to merge 1 commit into from

Conversation

phoenixlzx
Copy link

This resolves #1 and easier to work with multiple copies/projects.
Don't forget to add .env to .gitignore 🤣
I'm new to Golang so please criticize if I'm not doing it right :D

@nicolas-graves
Copy link
Owner

nicolas-graves commented Jun 30, 2023

Ok, indeed I wrote the program under the hypothesis that a tool like direnv would be enough not to have the need to read a .env file directly. The suggested solution in the README was to use a .envrc with direnv that would put environment variables in the environment.

I have to think a bit more about this. The issue is that yes it will work when you use it as a local repo, but let's say we package it on some distros, then you would just have an executable file in $PATH, but were would you go find this .env ? In the current directory when you execute the command ?

I won't close it for the moment, but I'm not sure this would be the way to go in the case we package it later. In any case, thanks for investigating the issue, I may need to put more enphasis on the direnv part in the README.

@phoenixlzx
Copy link
Author

I only added the action to read the variables from .env, it should not prevent you from using the variables set by other tools.

@nicolas-graves
Copy link
Owner

This I understand. My point is circumvent what should be handled by the package and what should not. I'm going to take a few days with this at the back of my mind.

@nicolas-graves
Copy link
Owner

nicolas-graves commented Jul 2, 2023

I think a program should not read a file in such a hidden way. Maybe we can add a cli option to read the environment from a file, but it shouldn't be hidden like this.

For the moment, I'll close this PR. However, I understood that some experienced errors might come from the fact that the environment is not properly set, thanks for the debugging in the issue #1 . I've added some error handling in this case in commit f575b78.

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.

Stuck in 'Uploading LFS objects'
2 participants