Skip to content

Conversation

Donbur4156
Copy link
Contributor

Pull Request Type

  • Feature addition
  • Bugfix
  • Documentation update
  • Code refactor
  • Tests improvement
  • CI/CD pipeline enhancement
  • Other: [Replace with a description]

Description

setting "last_call_time" in _fire() changes it for the OrTrigger but not for the relevant Triggers.
The function set_last_call_time() set it for normal Trigger and in case of OrTrigger to the "current_trigger".
The current_trigger is the trigger for which triggers next.

Changes

  • add var current_trigger: BaseTrigger in class OrTrigger
  • extract evaluate current_trigger to function _set_current_trigger
  • add function set_last_call_time to BaseTrigger and modified it in OrTrigger

Related Issues

#1437

Test Scenarios

@listen()
async def on_startup():
    cron_print.start()

@Task.create(
    OrTrigger(
        IntervalTrigger(2),
        TimeTrigger(hour=19, minute=1, utc=False),
        DateTrigger(target_datetime=datetime(day=3,month=6,year=2023, hour=19, minute=1, second=30)),
    )
)
async def cron_print():
    print(datetime.now().strftime("%H:%M:%S.%f")[:-3])

set the timings in near future and probably comment out some triggers

Python Compatibility

  • I've ensured my code works on Python 3.10.x
  • I've ensured my code works on Python 3.11.x

Checklist

  • I've run the pre-commit code linter over all edited files
  • I've tested my changes on supported Python versions
  • I've added tests for my code, if applicable
  • I've updated / added documentation, where applicable

Donbur4156 and others added 2 commits June 3, 2023 18:54
setting "last_call_time" in _fire() change it for the OrTrigger but not for the relevant Triggers.
the function set_last_call_time() set it for normal Trigger and in case of OrTrigger to the "current_trigger".
The current_trigger is the trigger for which triggers next.
Copy link
Member

@silasary silasary left a comment

Choose a reason for hiding this comment

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

Good catch

@LordOfPolls
Copy link
Contributor

Looks good, and seems to be working fine. One nitpicky suggestion

Co-authored-by: LordOfPolls <dev@lordofpolls.com>
Signed-off-by: Donbur4156 <janpfister@t-online.de>
@LordOfPolls LordOfPolls merged commit 5af62fe into interactions-py:unstable Jun 13, 2023
@AstreaTSS AstreaTSS mentioned this pull request Jul 13, 2023
13 tasks
@Donbur4156 Donbur4156 deleted the fix_ortrigger_fire_endless branch July 28, 2023 14:10
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