8000 [Scheduler] Normalize `TriggerInterface` as `string` by valtzu · Pull Request #59679 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Scheduler] Normalize TriggerInterface as string #59679

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 7, 2025
Merged

[Scheduler] Normalize TriggerInterface as string #59679

merged 1 commit into from
Feb 7, 2025

Conversation

valtzu
Copy link
Contributor
@valtzu valtzu commented Feb 3, 2025 •
edited
Loading
Q A
Branch? 7.3
Bug fix? yes
New feature? yes
Deprecations? no
Issues Fix #53562
License MIT

Fix the issue with Scheduler & Serializer like @kbond described in #53562 (comment).

Fixes the following error:

Could not decode stamp: The type of the "trigger" attribute for class "Symfony\Component\Scheduler\Generator\MessageContext" must be one of "Symfony\Component\Scheduler\Trigger\TriggerInterface" ("array" given).

@valtzu valtzu requested a review from kbond as a code owner February 3, 2025 16:04
@carsonbot carsonbot added this to the 6.4 milestone Feb 3, 2025
@valtzu valtzu mentioned this pull request Feb 3, 2025
Closed
nicolas-grekas
nicolas-grekas reviewed Feb 4, 2025
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

No opinion on the patch itself. From afar, adding a new class + service looks like a new feature to me but I'm not much into the topic.

@valtzu
Copy link
Contributor Author
valtzu commented Feb 4, 2025 •
edited
Loading

No opinion on the patch itself. From afar, adding a new class + service looks like a new feature to me but I'm not much into the topic.

Thanks for the review @nicolas-grekas, I have applied all suggestions. Let's wait for @kbond's take on the patch then. I would hope this makes it to 6.4, but since the issue (not being able to use Messenger transports using Symfony Serializer with Scheduler) has existed already over a year, waiting a little bit more (for 7.3) doesn't make a huge difference for me.

@valtzu valtzu force-pushed the fix-scheduler-stamp branch from 9f6583d to 20f676a Compare February 4, 2025 17:20
kbond
kbond approved these changes Feb 5, 2025
View reviewed changes
Copy link
Member
@kbond kbond left a comment

Choose a reason for hiding this comment

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

This looks good to me!

I'm not sure on 6.4 vs 7.3. This does fix a bug but does so by adding a new service which feels like a feature.

@kbond
Copy link
Member
kbond commented Feb 5, 2025

Adding new classes or non private methods

From the maintenance doc. Strictly following this means this is a feature.

@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.3 Feb 5, 2025
@valtzu valtzu changed the base branch from 6.4 to 7.3 February 5, 2025 18:45
@valtzu valtzu force-pushed the fix-scheduler-stamp branch from 20f676a to 3aa47e0 Compare February 5, 2025 18:46
@valtzu
Copy link
Contributor Author
valtzu commented Feb 5, 2025

Ok, thanks, rebased on 7.3 as a new feature.

@fabpot
Copy link
Member
fabpot commented Feb 7, 2025

Thank you @valtzu.

@fabpot fabpot merged commit 277486a into symfony:7.3 Feb 7, 2025
8 of 11 checks passed
@fabpot fabpot mentioned this pull request Feb 7, 2025
Closed
@valtzu valtzu mentioned this pull request Feb 7, 2025
Open
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@fabpot fabpot fabpot approved these changes

@kbond kbond kbond approved these changes

Assignees
No one assigned
Labels
Bug Scheduler Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Scheduler][Messenger] Message can't be redispatched with Symfony serializer in between
5 participants
@valtzu @kbond @fabpot @nicolas-grekas @carsonbot
0