bugy / script-server

Web UI for your scripts with execution management
Other
1.52k stars 244 forks source link

Extend repeat schedule functionality #676 #677

Closed UrekD closed 12 months ago

UrekD commented 1 year ago

Implement functionality discussed in #676:

bugy commented 1 year ago

Hi @UrekD sorry for a late response. Unfortunately I'm a bit busy this week, so don't have time to review. Will try to check the pull request within the next several days. And thanks a lot for the pull request!

UrekD commented 1 year ago

Hi @UrekD I think we are almost there :) I have only one big concern, about making this new feature optional

Hey @bugy, how so? I'm guessing maybe from an administrative standpoint? As the functionality shouldn't in no way hinder the functionality as it was before, if you don't select anything else. I see the point about not requiring a value for never ending schedules that was oversight on my end, which actually thought about not implementing in a current way but not for that reason :D I'll address the new stuff tomorrow, thanks!

UrekD commented 12 months ago

Sorry for delay and lots of previous comments Looks good to me now, thanks a lot for the contribution! 👍 🎉

No worries, thank you!

vnghia commented 6 months ago

Can we avoid overwriting the configuration file ? Because I am using an Infra-as-code approach and everything is generated by some kind of code and set to read-only afterward. So this failed on my server.

https://github.com/bugy/script-server/blob/7ea018023532d42b101fef2ebf7b49ded2ec6b1a/src/scheduling/schedule_service.py#L155-L158

Maybe add a is_forever attribute to the schedule_config and we won't touch the file again if it has is_forever. I can send a PR for that.

UrekD commented 6 months ago

Can we avoid overwriting the configuration file ? Because I am using an Infra-as-code approach and everything is generated by some kind of code and set to read-only afterward. So this failed on my server.

https://github.com/bugy/script-server/blob/7ea018023532d42b101fef2ebf7b49ded2ec6b1a/src/scheduling/schedule_service.py#L155-L158

Maybe add a is_forever attribute to the schedule_config and we won't touch the file again if it has is_forever. I can send a PR for that.

Yeah that sounds good maybe better calling it allow_write. Personally I'd use a modified image for deployment, as with the new var implementation you will also need to handle a schedule that is set to count for it to not fire forever if it can't keep track of the count, not sure if it's worth the hassle.

vnghia commented 6 months ago

I think I only need it not to touch the config files that are persistent, i.e files with no expiration regardless time or execution count. The file with expiration can still be overwritten because usually these files are generated dynamically while the former can be generated and managed with Infra-as-code.

So we can check if the file will expire and overwrite it or just leave it there. The execution count will still be kept track inside the original file.