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

Installation: Day 1 improvements to settings #836

Merged
merged 49 commits into from
Feb 14, 2025

Conversation

peteski22
Copy link
Contributor

@peteski22 peteski22 commented Feb 10, 2025

What's changing

Improvements to Lumigator's day 1 installation story (settings).

  • Remove use of .env file directly (.env.template not needed)
  • VS Code terminal issue around default vars is now moot
  • Updated docs
  • Use .default.conf and optional user.conf for configuration of app
  • Allow default settings and user supplied settings to be combined, preferring user settings

Closes #812

How to test it

Steps to test the changes:

Defaults:

make start-lumigator
# No user configuration found, default will be used: '.default.conf'
cat .default.conf | grep DEPLOYMENT_TYPE # should show 'local'
curl -s 'http://localhost:8000/api/v1/health/' | jq '.deployment_type' # should show 'local'
cat build/.env # Should show defaults
make stop-lumigator

Then add a setting via the user config override:

rm user.conf # Sanity check
echo DEPLOYMENT_TYPE=development > user.conf
make start-lumigator
# "Found user configuration: 'user.conf', overrides will be applied"
cat .default.conf | grep DEPLOYMENT_TYPE # should show 'local'
cat user.conf | grep DEPLOYMENT_TYPE # should show 'development'
curl -s 'http://localhost:8000/api/v1/health/' | jq '.deployment_type' # should show 'development'
cat build/.env # Should show aggregated settings
make stop-lumigator

Additional notes for reviewers

Anything you'd like to add to help the reviewer understand the changes you're proposing.

I already...

  • Tested the changes in a working environment to ensure they work as expected
  • Added some tests for any new functionality
  • Updated the documentation (both comments in code and product documentation under /docs)
  • Checked if a (backend) DB migration step was required and included it if required

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 10, 2025
config.default.yaml Outdated Show resolved Hide resolved
@peteski22 peteski22 marked this pull request as ready for review February 12, 2025 13:56
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@agpituk agpituk left a comment

Choose a reason for hiding this comment

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

thanks for all the changes, great work Peter!

@peteski22 peteski22 enabled auto-merge (squash) February 14, 2025 11:16
@peteski22
Copy link
Contributor Author

thanks for all the changes, great work Peter!

No worries, it was a team effort with the conversations and contributions. Thanks to @macaab26 too! 🥳

@peteski22 peteski22 merged commit fa3a2a0 into main Feb 14, 2025
17 checks passed
@peteski22 peteski22 deleted the 812/settings/day-1-install-docker branch February 14, 2025 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify installation: Centralise YAML configuration for Docker
3 participants