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

Scheduler trigger interval as environment variable setting #3591

Conversation

PopaRares
Copy link
Contributor

Description

Added a new setting called SCHEDULER_TRIGGER_INTERVAL that sets the LoopTimeTrigger's sleep time between scheduler calls. Value is of type float. Default value is kept at 10 seconds.
Example call:

 SCHEDULER_TRIGGER_INTERVAL=3 ./scripts/dev.sh

How Has This Been Tested?

Set the SCHEDULER_TRIGGER_INTERVAL to some numerical value (default value is 10 seconds).

SCHEDULER_TRIGGER_INTERVAL=3.5 ./scripts/dev.sh

Checklist

  • The PR is tagged with proper labels (bug, enhancement, feature, documentation)
  • I have performed a self-review of my own code
  • I have added unit tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

cc:
@adelcast
@hugabora

@PopaRares
Copy link
Contributor Author

@wangxiaoyou1993 I've been looking at the classes LoopTimeTrigger and TimeTrigger and I have some light concerns. Should the trigger_interval property be held in the TimeTrigger class? How things work currently is that LoopTimeTrigger does the waiting and TimeTrigger triggers the scheduler; going by that logic, the trigger_interval property should be a property of the entity that does the actual waiting: LoopTimeTrigger. Expandig from this, what are the advantages of having LoopTimeTrigger and TimeTrigger be 2 separate classes? TimeTrigger is only used with the loop part attached so maybe a good solution would be to merge these two? Let me know what you guys think!

@wangxiaoyou1993
Copy link
Member

@wangxiaoyou1993 I've been looking at the classes LoopTimeTrigger and TimeTrigger and I have some light concerns. Should the trigger_interval property be held in the TimeTrigger class? How things work currently is that LoopTimeTrigger does the waiting and TimeTrigger triggers the scheduler; going by that logic, the trigger_interval property should be a property of the entity that does the actual waiting: LoopTimeTrigger. Expandig from this, what are the advantages of having LoopTimeTrigger and TimeTrigger be 2 separate classes? TimeTrigger is only used with the loop part attached so maybe a good solution would be to merge these two? Let me know what you guys think!

class LoopTimeTrigger inherits the class TimeTrigger. It's just one way to implement the time trigger. It's just to make it extensible if we want to add another implementation for TimeTrigger in the future.

@PopaRares PopaRares changed the title Dev/rpopa/scheduler trigger interval as env var setting Scheduler trigger interval as environment variable setting Sep 27, 2023
@wangxiaoyou1993 wangxiaoyou1993 added the enhancement Polish or UX improvements label Sep 29, 2023
@wangxiaoyou1993 wangxiaoyou1993 merged commit 4bda3b8 into mage-ai:master Sep 29, 2023
5 checks passed
@wangxiaoyou1993 wangxiaoyou1993 mentioned this pull request Oct 3, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Polish or UX improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants