-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[8.x] Rename some queue configurations #32728
Conversation
|
What if instead of Option 1:
Option 2:
I know the issue with |
Instead of immediately renaming the methods, would you be open to adding the new methods, calling the methods they’re replacing internally, and marking the old methods I know a major version allows for breaking changes, but deprecating them gives developers a more gradual ramp to update their code. Alternatively, doing it this way would even allow you to introduce the new methods in a minor release in 7.x, add the deprecation notice, and remove in 8.0 if you really wanted to target that release. It’s only a ~3 month window from deprecation to removal, though. |
I think |
agree with @trevorgehman |
Hey everyone,
We also don't want to use retry-delay because we want to keep the term "delay" used only for pushing jobs to be queued later. As for deprecation notices. I think the change can be done via find-and-replace in your code editor so it won't make the upgrade too much difficult. So imo I think we can go ahead and rename the method/properties in 8.x |
I think most changes can be some via find and replace, I don’t dispute that. It’s about deprecating methods more thoughtfully, and providing some notice - outside of a PR that not everybody is going to see - to give people a chance to deal with the deprecation. |
* | ||
* @var int | ||
*/ | ||
public $retryAfter; | ||
public $backoff; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Others have already commented about the nomenclature of backoff
here.
I just want to add that backoff
has a certain connotation of gradually backing off, so that each time it fails the delay is increased 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello 👋🏼
I know no one asked for my opinion here, but what triggered me writing this comment is that, now that I was faced with the delay/backoff rename in a project and researched how this came about, I was under the same impression and thought "this doesn't feel right". As you said: "gradually backing off" was also my expectation
But, IMHO, turns out we're mostly just so used to the (indeed very common place) e.g. "exponential backoff" that it's easy to assume (even myself) the "backoff" equals "exponential backoff".
But in fact there are just different types of backoff one can do:
- linear
- constant
- exponential
- …
My 2c FWIW 😄
This PR does the following:
--delay
option of thequeue:work
command.--backoff
option to the command to replace--delay
.$retryAfter
andretryAfter()
to$backoff
andbackoff()
on jobs, mailers, notifications, and listeners.$timeoutAt
in jobs, notifications, and listeners to$retryUntil
to match the name of the alternativeretryUntil()
method and not conflict with the$timeout
property.Marked as draft as I'm going to check there are no other confusing configurations on Monday. Just wanted to make it public for feedback.