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

Allow configuration overrides #383

Merged
merged 8 commits into from
Nov 5, 2024
Merged

Conversation

PGijsbers
Copy link
Collaborator

@PGijsbers PGijsbers commented Oct 31, 2024

This PR adds documentation (and the possibility) to allow overrides for the src/config.toml configuration and the .env configuration. This is useful, because then you can customize your own deployment (also production), without affecting files which are checked into git, which means:

  • significantly less risk of accidentally checking in your custom configuration (potentially leaking secrets)
  • easier to pull in updates, as previously changed configurations would lead to merge conflicts or otherwise needed to be undone before a pull

I also removed the passing around of environment variables between all docker images, as the relevant variables were already also passed through the environment field. Now containers are only aware of the variables they need.

How to Test

Change some configurations in src/config.override.toml and override.env as described in the documentation with some custom options (whatever you are already using, or something new just to test). up the containers, poke around, down the containers.

@PGijsbers PGijsbers marked this pull request as ready for review October 31, 2024 15:15
Copy link
Collaborator

@AdrianAlcolea AdrianAlcolea left a comment

Choose a reason for hiding this comment

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

As we are already talking about that I close the revision here and we can start another one ;p

docker-compose.yaml Show resolved Hide resolved
docker-compose.yaml Show resolved Hide resolved
co-authored by: Adrian Alcolea <AdrianAlcolea@users.noreply.github.com>
@PGijsbers
Copy link
Collaborator Author

I also had the PYTHON_PATH... I think we should probably just bake that into the docker image, though? Since it's required for the image to function even if no code is mounted... otherwise, things seem to work. All connectors are able to import data now and I can search both directly with the API and through the elastic search endpoints.

Copy link
Collaborator

@AdrianAlcolea AdrianAlcolea left a comment

Choose a reason for hiding this comment

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

Done, everything works for me now =)

Copy link
Collaborator

@Taniya-Das Taniya-Das left a comment

Choose a reason for hiding this comment

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

works well for me now.

@PGijsbers PGijsbers merged commit f784e9b into develop Nov 5, 2024
1 check passed
@PGijsbers PGijsbers deleted the feature/simplify-release branch November 5, 2024 13:47
@PGijsbers PGijsbers mentioned this pull request Nov 6, 2024
4 tasks
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.

3 participants