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

Docker Container logic updates #100

Closed
wants to merge 6 commits into from
Closed

Docker Container logic updates #100

wants to merge 6 commits into from

Conversation

rursache
Copy link

  • The docker container now runs indefinitely with a sleep delay between runs
  • Configure the sleep delay in the config.json or via the --sleep-interval argument
  • The container now takes a mounted volume from which it loads the config.json. Cleaner and more elegant + fixeshttps://github.com/Running from container cannot use config file. #64
  • Updated the github action that builds the container to builds other archs as well

@nanos
Copy link
Owner

nanos commented Feb 12, 2024

Thanks for your PR @rursache !

I appreciate your work, but it's worth saying that the infinite loop on the script doesn't apply only to docker, but also to other forms of execution.

The biggest problem with this is that it makes it incompatible with GitHub Actions. The secondary problem is that I don't personally like running it like this, as it doesn't deal well with failures: Imo starting 'from scratch' from time to time has it's advantages, when run outside a docker container.

I also think that the first two and last two bullet points would be better served with two separate PRs to be honest (among others because I'd merge that in right away).

@rursache
Copy link
Author

rursache commented Feb 12, 2024

while i understand what you say, i made these changes because first of all i want to run this script in a docker container, set and forget it. i see no benefit of running it manually. the loop "starts from scratch" if it crashes due to --restart unless-stopped in docker run/compose param. also its not resource intensive so it can be ran locally on a machine instead of semi-abusing GH actions.

a new flag (--docker) could be added to the python script to do this loop only when ran as a container but i don't plan to implement and maintain that. that would fix everything you mentioned imo.

good luck and all the best

@nanos nanos closed this Feb 13, 2024
@benyafai
Copy link
Contributor

@rursache Check our PR #102

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