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

add support for hot reloading the config.yaml #100

Merged
merged 5 commits into from
Oct 11, 2024

Conversation

arnesetzer
Copy link
Contributor

@arnesetzer arnesetzer commented Aug 17, 2024

As discussed in #99 a try to enable hot reloading for config (and therefore datasets) without rebuilding the docker image again.

What changed:
A watchdog (/app/watcher.py) is etablish via supervisor. On a config.yaml add or change (CAVE: Due to docker limitations it won't happen if the inode changes, see: https://medium.com/@jonsbun/why-need-to-be-careful-when-mounting-single-files-into-a-docker-container-4f929340834) the watchdog is triggerd an kills the uwsgi and flushes memcached.

@arnesetzer
Copy link
Contributor Author

@ajnisbet Any feedback? 🙈

@ajnisbet
Copy link
Owner

ajnisbet commented Oct 2, 2024

Thanks for this Arne!

It's no my radar! I've been on holiday the last month and there's a few things I'll need to tweak before merging, here's a quick review (for my later reference):

  • Use subprocess instead of os.system
  • Stop and start uwsgi with supervisord instead of killing it
  • Add warm cache command
  • Watch both config.yaml and example-config.yaml

@arnesetzer
Copy link
Contributor Author

Hi, thanks for the feedback.

* [ ]  Use `subprocess` instead of `os.system`

Done, now using pymemcache to avoid any system manipulation.

* [ ]  Stop and start `uwsgi` with `supervisord` instead of killing it

I'm a little bit puzzled how to achieve this. I played around with the supervisorctl but this requires some serious changes in the supervisord configuration (

* [ ]  Add warm cache command

Already done 😄

* [ ]  Watch both `config.yaml` and `example-config.yaml`

Also done now.

@ajnisbet ajnisbet changed the base branch from master to dev October 11, 2024 18:56
@ajnisbet ajnisbet merged commit 9d51071 into ajnisbet:dev Oct 11, 2024
@ajnisbet
Copy link
Owner

Thanks for this work @arnesetzer !

Here's the final working code if you're interested: d1fc1a7

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.

2 participants