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

issue #221: .env.secrets & .env.config #222

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

k-allagbe
Copy link
Member

No description provided.

@k-allagbe k-allagbe requested a review from a team as a code owner December 10, 2024 16:42
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Dec 10, 2024
@k-allagbe k-allagbe marked this pull request as draft December 10, 2024 16:43
@k-allagbe k-allagbe force-pushed the 221-secretsenv-and-configenv branch 2 times, most recently from 0d92dfe to 30f3957 Compare December 10, 2024 16:47
@k-allagbe k-allagbe force-pushed the 221-secretsenv-and-configenv branch from 30f3957 to f5c3bd0 Compare December 10, 2024 16:48
@k-allagbe
Copy link
Member Author

@ThomasCardin this works perfectly locally. I have issues running it in a docker container however. The /analyze route in particular fails with a 401 error.

@k-allagbe k-allagbe marked this pull request as ready for review December 10, 2024 16:58
Comment on lines 12 to 15
FERTISCAN_DB_URL=

# Blob Storage
FERTISCAN_STORAGE_URL=
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it would be possible to store both the username and password instead of the full URL for the secrets.env.template file? Full url's are quite hard to manage with the git-secrets tool

Choose a reason for hiding this comment

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

I agree we should be splitting this up and building the string in code at runtime if we need to use the string

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Note that the modifications to the code are so that the tests no longer require secrets.

Copy link
Member

Choose a reason for hiding this comment

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

@k-allagbe Do we want to implement the proposed changes to include the username and password in the .env.secrets.template file, or is there a technical constraint? I ask because I still see FERTISCAN_DB_URL in the .env.secrets.template file, and I don’t see any updates in the code showing that the database username and password are being used to construct the connection string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@k-allagbe k-allagbe marked this pull request as draft January 6, 2025 13:02
@k-allagbe k-allagbe force-pushed the 221-secretsenv-and-configenv branch from 1078236 to 8e6ed91 Compare January 6, 2025 14:09
@github-actions github-actions bot added the tests label Jan 6, 2025
@k-allagbe k-allagbe force-pushed the 221-secretsenv-and-configenv branch 2 times, most recently from ee1c5f1 to f1d31c5 Compare January 6, 2025 14:33
@k-allagbe k-allagbe marked this pull request as ready for review January 6, 2025 15:00
@k-allagbe k-allagbe force-pushed the 221-secretsenv-and-configenv branch from f1d31c5 to 8856305 Compare January 6, 2025 15:18
@k-allagbe k-allagbe changed the title issue #221: secrets.env & config.env issue #221: .env.secrets & .env.config Jan 6, 2025
@k-allagbe k-allagbe force-pushed the 221-secretsenv-and-configenv branch from 8856305 to 059ccce Compare January 6, 2025 15:26
@k-allagbe
Copy link
Member Author

@ai-cfia/fertiscan please review when you can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As a dev, I want to separate environment variables into .env.secrets and .env.config
3 participants