[BUGFIX] Respect changed stopIfNecessary() signature#34
Open
sbuerk wants to merge 1 commit intophp-enqueue:masterfrom
Open
[BUGFIX] Respect changed stopIfNecessary() signature#34sbuerk wants to merge 1 commit intophp-enqueue:masterfrom
stopIfNecessary() signature#34sbuerk wants to merge 1 commit intophp-enqueue:masterfrom
Conversation
`\Illuminate\Queue\Worker::stopIfNecessary()`
method signature has changed from
```php
protected function stopIfNecessary(
WorkerOptions $options,
$lastRestart,
$job = null
) {}
```
to
```php
protected function stopIfNecessary(
WorkerOptions $options,
$lastRestart,
$startTime = 0,
$jobsProcessed = 0,
$job = null
) {}
```
between `7.x` and `8.x` of depending package
`illuminate/queue` [1][2] and a valid breaking
change between major versions.
The changed signature has not been respected and
mitigated with allowing `illuminate/queue:8.x`,
leading to following exception in case a interop
is used:
```
Unsupported operand types: float - \CustomJobObject (0)
Illuminate\Queue\Worker->stopIfNecessary(
Object(Illuminate\Queue\WorkerOptions),
NULL,
Object(FES\JobQueue\Job\Job)
)
```
To mitigate the error, this change:
* Modifies `\Enqueue\LaravelQueue\Worker` to track the
`start_time` and the number of `processed` jobs with
an configured interop, mainly in `Worker:deamon()`
and `Worker::runNextJob()`.
* Modifies `Worker::onPostMessageReceived()` to call
`stopIfNecessary()` passing the job as last argument
and providing the new inbetween options to mitigate
the error message.
This package `enqueue/laravel-queue` changed the
supported versions of the dependency package
`illuminate/queue` on `patchlevel` releases and
considerable violating semver and making it now
impossible to release this fix for the target
version with older `illuminate/queue` constraints.
Following versions would need fixes also changing
the illuminate/queue constraints:
```
* 0.10.3 -> added illuminate/queue:^8.0 support
and failed to repsect the changed
method signature for 8, or better to
say to deal with different argument
signatures <8.0 and >= 8.0.
* 0.10.6 -> added `illuminate/queue:^9.0` support
* 0.10.19 -> removed `illuminate/queue` support for
^6.0, ^7.0 and ^8.0.
* 0.10.20 -> removed `^9.0` and added `^11.0` support
for `illuminate/queue`
* 0.10.21 -> removed `^10.0` and added `^12.0` support
```
Adding additional major version support of dependencies
is not considerable breaking, and not respecting the
method change can be taken as `bug`. However, dropping
major version support of dependencies is considerable
breaking and **must** have gained a `major` version
bump for this package already.
The fix can now only be made on top, not fixing it for
the older releases and consumers requiring the fix with
`illuminate/queue` versions `^8.0 | ^9.0 | ^10.0`.
[1] laravel/framework#33449
[2] laravel/framework@441a85a
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
\Illuminate\Queue\Worker::stopIfNecessary()method signature has changed from
to
between
7.xand8.xof depending packageilluminate/queue[1][2] and a valid breakingchange between major versions.
The changed signature has not been respected and
mitigated with allowing
illuminate/queue:8.x,leading to following exception in case a interop
is used:
To mitigate the error, this change:
Modifies
\Enqueue\LaravelQueue\Workerto track thestart_timeand the number ofprocessedjobs withan configured interop, mainly in
Worker:deamon()and
Worker::runNextJob().Modifies
Worker::onPostMessageReceived()to callstopIfNecessary()passing the job as last argumentand providing the new inbetween options to mitigate
the error message.
This package
enqueue/laravel-queuechanged thesupported versions of the dependency package
illuminate/queueonpatchlevelreleases andconsiderable violating semver and making it now
impossible to release this fix for the target
version with older
illuminate/queueconstraints.Following versions would need fixes also changing
the illuminate/queue constraints:
Adding additional major version support of dependencies
is not considerable breaking, and not respecting the
method change can be taken as
bug. However, droppingmajor version support of dependencies is considerable
breaking and must have gained a
majorversionbump for this package already.
The fix can now only be made on top, not fixing it for
the older releases and consumers requiring the fix with
illuminate/queueversions^8.0 | ^9.0 | ^10.0.[1] laravel/framework#33449
[2] laravel/framework@441a85a