-
Notifications
You must be signed in to change notification settings - Fork 27
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
feature: allow scheduling of scheduled tasks #290
feature: allow scheduling of scheduled tasks #290
Conversation
Doesn't really matter here but I wanted to point it out anyways. |
can we not overwrite the data of symfony scheduler? |
I don't think so. The only way to do this is to dispatch the message manually to the message bus. The scheduler keeps information about lastRun in a local variable. And with this PR shopware/shopware#5456 it also writes it to cache. But it will never update the DB. On the other hand when using the symfony scheduler, the status will also never change to queued/failed. It will always remain in the scheduled state. This is because the bridge never updates this value. It just checks for it. |
Maybe it should be dispatched immediately inside the |
Nevermind, I was wrong about this altogether xD. It will always run, even if it is in the queued or failed state. The only exception is the inactive state. See here: https://github.com/shopware/shopware/blob/3d2e531639877ff0d839b98fc6081beec0cd5ebf/src/Core/Framework/MessageQueue/ScheduledTask/SymfonyBridge/ScheduleProvider.php#L46 |
So the current state of the scheduling code in this PR is correct?
Is this intended behaviour?
Sounds like new PR I can create :D |
Personally I would not put too much thought into this. Symfony scheduler bridge is currently marked experimental. There are some things that are quite broken about it currently but no one really seems to care (I pointed some of those things out in my PR but it was just merged without even talking about them :D). Scheduler Bridge will break other thinks in FroshTools anyways (like overdue scheduled tasks). So I would just merge it for now. What do you think @shyim ? |
...urces/app/administration/src/module/frosh-tools/component/frosh-tools-tab-scheduled/index.js
Outdated
Show resolved
Hide resolved
…ent/frosh-tools-tab-scheduled/index.js Co-authored-by: Felix Schneider <69912882+schneider-felix@users.noreply.github.com>
No description provided.