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

Environment variables documentation #1089

Merged
merged 3 commits into from
Sep 19, 2024
Merged

Conversation

MukuFlash03
Copy link
Contributor

Creating a consolidated documentation for environment variables that are used across the repositories of OpenPATH repo.

Initially starting out with repositories involved in the Redesign changes (PR, Issue) - e-mission-server, join-page, admin-dash, public-dash

Copy link
Contributor

Choose a reason for hiding this comment

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

the whole point of having searches instead of listing out a table was because the table will quickly become obsolete and gives people a false sense of knowledge. Please remove.

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

@MukuFlash03 I wasn't complaining about the format (table versus list) in which the existing variables are listed. I think we should not list the variables at all. The list of variables will become obsolete as the code changes, and at this point, we are not signing up to keep it maintained.

As a concrete example, I just checked in e-mission/e-mission-server#980 so the list of push variables in this PR is already obsolete. This should either be autogenerated, or it should be removed entirely.

If included, then it won't be up-to-date if we update / remove variables and will need to update this everytime.
Hence removing the specific names of environment variables.
@MukuFlash03
Copy link
Contributor Author

Right, I did see the Push config changes and yes it would keep rendering the doc obsolete. I had this in mind when I removed the table. But in the flow of updating the documentation, I just missed the whole point and re-added it as a list.

Removed references of individual variable names. Search results mentioned only now.

Comment on lines +14 to +15
- Use this [search](https://github.com/search?q=repo%3Ae-mission%2Fe-mission-server+%2F.environ.get%5C%28%5C%22%5BA-Z%5D%2F&type=code) to obtain usage of `Overpass, Nominatim, Geofabrik` environment variables in the [e-mission-server](https://github.com/e-mission/e-mission-server) repository.
- Use this [search](https://github.com/search?q=repo%3Ae-mission%2Fe-mission-server+geofabrik&type=code) to view some variables that are set using GitHub secrets in the [e-mission-server](https://github.com/e-mission/e-mission-server) GitHub workflows.
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the difference between these two searches. the second seems to look for geofabrik, but you also list geofabrik in the first search.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With reference to Geofabrik, the 1st search lists GEOFABRIK_OVERPASS_KEY, GEOFABRIK_QUERY_URL.

In addition to these, the 2nd search also lists GFBK_KEY which is used as a suffix to GEOFABRIK_QUERY_URL in emission/integrationTests/docker-compose.yml.
This variable is set while running the docker compose file in a workflow file using a GitHub secret GEOFABRIK_API which is different from GEOFABRIK_OVERPASS_KEY.

This doesn't show up while searching for environ.get() since we are using it directly in the docker-compose file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm. I am not sure we need to list the secrets for GitHub workflows, given that we don't really want people to be working on forks. However, I guess it could be useful at some point. The problem, though, is that if we add another secret to the GH workflow, it won't show up with this search.

We have gone back and forth enough times on this that I think that I am not going to force this change. But I fully expect it to bitrot within the next 5 years, at which point we should remove it. Let's see if I am correct!

@shankari
Copy link
Contributor

Squash-merging to avoid commit churn

@shankari shankari merged commit 29cc1e6 into e-mission:master Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Tasks completed
Development

Successfully merging this pull request may close these issues.

2 participants