Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Scheduler] Throw error on duplicate schedule provider service registration on the schedule name #60253

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

Open
wants to merge 2 commits into
base: 7.3
Choose a base branch
Loading
from

Conversation

adrianrudnik
Copy link
Contributor

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues See below
License MIT

The way the schedule providers work, either by tagging a service or using the AsSchedule attribute, can lead to a scenario where multiple ScheduleProviders register for the same schedule name (e.g. default).

The problem arises when the CompilerPass simply overwrites the previously registered one without throwing a warning, a notice or anything else. The problem would be that a full ScheduleProvider would simply not register and therefore not run.

I decided to use the InvalidArgumentException from DI, as I could not get a trigger_error on E_USER_WARNING to show anything in a dev environment, in case I miss a scenario where this would actually be a desirable use case.

Not sure if the test is complete enough for this scenario. I tried to base it on others found in HttpKernel.

Also not sure if this falls under "feature" or "bug fix", so I marked it as a feature above.

@adrianrudnik
Copy link
Contributor Author

A similar Q&A discussion from last year can be found here

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.

Nice, this makes sense. I agree, let's call this a feature. Can you add an entry to the scheduler's CHANGELOG?

@adrianrudnik
Copy link
Contributor Author

@kbond Sure! Hope the message within the changelog is OK and detailed enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.