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

chore: Handle missing environment variables for local dev #433

Merged
merged 4 commits into from
Jul 8, 2024

Conversation

microamp
Copy link
Contributor

@microamp microamp commented Jul 7, 2024

Fixes #432

I could've added SOURCE_REPO_URL to .env in the documentation instead of changing docker-compose.yml, but decided to treat it like MATOMO_URL, MATOMO_SITE_ID, etc. at the end. Let me know.

@microamp microamp requested a review from kesara July 7, 2024 22:47
@@ -88,7 +88,7 @@ services:
PRIMARY_HOSTNAME: ${HOST_NAME?missing main hostname (e.g., localhost for development)}
INTERNAL_HOSTNAMES: "web"
DEBUG: ${DEBUG:-0}
SOURCE_REPO_URL: ${SOURCE_REPO_URL:?missing source repository URL}
SOURCE_REPO_URL: ${SOURCE_REPO_URL}
Copy link
Member

Choose a reason for hiding this comment

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

It is better to have a safeguard because SOURCE_REPO_URL is required for the proper functionality of the services.

Copy link
Member

@kesara kesara left a comment

Choose a reason for hiding this comment

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

Unlike MATOMO_*, SOURCE_REPO_URL is required for getting data. So better to have this one .env.

@kesara
Copy link
Member

kesara commented Jul 7, 2024

Note that SOURCE_REPO_URL for production service is SOURCE_REPO_URL: "https://github.com/ietf-tools/". So this could be the default value as well.

@microamp
Copy link
Contributor Author

microamp commented Jul 8, 2024

Note that SOURCE_REPO_URL for production service is SOURCE_REPO_URL: "https://github.com/ietf-tools/". So this could be the default value as well.

I noticed that that's what it says in the Helm chart.

SOURCE_REPO_URL: "https://github.com/ietf-tools/"

compared to the documentation, https://github.com/ietf-tools/bibxml-service/blob/docs/missing-env-vars/docs/howto/run-in-production.rst, that says

SOURCE_REPO_URL=https://github.com/ietf-tools/bibxml-service

Do you know which one's the correct one?

@microamp
Copy link
Contributor Author

microamp commented Jul 8, 2024

Unlike MATOMO_*, SOURCE_REPO_URL is required for getting data. So better to have this one .env.

I think SOURCE_REPO_URL is only being used on the about page, https://bib.ietf.org/about. Upon a quick look, there is a different set of variables related to dataset sources, DATASET_SOURCE_OVERRIDES, DEFAULT_DATASET_REPO_URL_TEMPLATE, etc. Please correct me if I'm wrong.

@microamp microamp requested a review from kesara July 8, 2024 00:42
@kesara
Copy link
Member

kesara commented Jul 8, 2024

Unlike MATOMO_*, SOURCE_REPO_URL is required for getting data. So better to have this one .env.

I think SOURCE_REPO_URL is only being used on the about page, https://bib.ietf.org/about. Upon a quick look, there is a different set of variables related to dataset sources, DATASET_SOURCE_OVERRIDES, DEFAULT_DATASET_REPO_URL_TEMPLATE, etc. Please correct me if I'm wrong.

Yeah, looks like it's defined in the settings file. This may be an opportunity to make things better. Ether get rid of the ENV or use that ENV in settings file?

@microamp microamp merged commit 7cf24aa into main Jul 8, 2024
2 checks passed
@microamp microamp deleted the docs/missing-env-vars branch July 8, 2024 01:45
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.

Local development setup: environment variables missing
2 participants