Allow queue jobs with batches #3974

winkelco · 2023-08-03T06:12:42Z

Please take note of our contributing guidelines: https://docs.laravel-excel.com/3.1/getting-started/contributing.html
Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.

1️⃣ Why should it be added? What are the benefits of this change?
allow import and export files with batches (New laravel feature for job batching)

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
No
3️⃣ Does it include tests, if possible?
No
4️⃣ Any drawbacks? Possible breaking changes?
No
5️⃣ Mark the following tasks as done:

Checked the codebase to ensure that your feature doesn't already exist.
Take note of the contributing guidelines.
Checked the pull requests to ensure that another person hasn't already submitted a fix.
Added tests to ensure against regression.
Updated the changelog

6️⃣ Thanks for contributing! 🙌

patrickbrouwers

I think the major problem with this remains that this is a Laravel 8+ feature, currently this package still supports a lot of older Laravel versions. I'm currently not at the point to create a 4.0 to deprecate all older versions.

If you could find a solution to work around the issue that the batchable trait doesn't exist in older laravel versions, it can be included. If not, we'll have to old of on this till the next major release.

patrickbrouwers · 2023-08-03T10:44:18Z

src/Jobs/AfterImportJob.php

@@ -36,6 +39,11 @@ public function __construct($import, Reader $reader)

    public function handle()
    {
+        // Determine if the batch has been cancelled...
+        if ($this->batch()->cancelled()) {


This would break if ShouldBatch isn't implemented and user would use the old queueing

patrickbrouwers · 2023-08-03T10:47:06Z

src/ChunkReader.php

+        // Check if the import class is batchable
+        if ($import instanceof ShouldBatch) {
+            return Bus::batch([
+                $jobs->toArray(),


This would create a batch with a chain, is that your intention? Would it make sense to add a concern to allow in parallel?

Yes, that's right

the concern is added so that the response is a PendingBatch and then you can add things like batch name, jobs at the end of the batch etc.

patrickbrouwers · 2023-08-03T10:50:11Z

src/Jobs/AfterImportJob.php

@@ -12,7 +15,7 @@

 class AfterImportJob implements ShouldQueue
 {
-    use Queueable, HasEventBus;
+    use Batchable, Queueable, HasEventBus, Dispatchable, ProxyFailures, InteractsWithQueue;


Batchable trait doesn't exists < Laravel 8

you're right. this would break compatibility with versions prior to 8

There are no security patches for laravel 8 anymore and 9 neither in 3 weeks. you shouldn't encourage people to stay on insecure versions.

I don't endorse nor encourage it. But the package is currently in a situation that dropping support for older versions has an impact on users. Unfortunately the harsh truth looking at packagist installs.

As I hinted dropping support means nothing for those user, they are already outdated versions of everything, I doubt they ever update anything only running composer install, I'm sure they don't even upgrade security patches, so why would they update your library either?

By dropping support in a minor they can still install the older package without the new features if they want with no issues, you do not need to create a major to drop support of a dependency with composer, and even if you do want to bump the version, then what's the issue? Those old users still can't upgrade (Composer will only install the last compatible version) and they likely won't be missing on anything, they'll still have the features they've been using for 6 years or they already would have upgraded their laravel which is quite easy to do in most cases.

Upgrading a major just has more friction because all the rest of the users need to bump the required major version manually as well as opposed to just composer upgrade

Unfortunately the majority of issues opened here are of people using older Laravel versions with the latest version of this package.
Features like queue batches are great, but I want to think it through carefully before releasing it to a lot of people. Having a package with a lot of users comes with a responsibility and less freedom. The last time I dropped older stuff I got huge backlash and way too many complaints. Not fun I can tell you. Therefore I decided to do a major release in the future which will - from that time on - only support the actively supported Laravel and PHP versions. I currently don't have time to finish the last bits of the planned 4.0 release, so any version dropping will also have to wait until then. I'm sorry if that's inconvenient to some, but that's also open source 😅

winkelco · 2023-08-25T21:23:06Z

ok i will try to find a solution

Tofandel · 2023-09-08T02:11:42Z

@patrickbrouwers You know deprecating laravel version (and even php version) does not require a major version bump with composer, it will still respect semver, that's because composer handles the version compatibility and will not let you install a newer version of the package if you are say on laravel 6 and you marked the newer package only laravel 8, it will select the latest compatible with your packages

In semver with composer, you only need to pay attention to your own breaking changes (eg: adding return types, changing signature, removing class etc)

If you don't believe me, have a look at symfony, they do this all the time

Btw laravel 8 has been EOL for 7 months already and php7.4 is also very much EOL, so it's really time to bump the dependency requirements, so we don't develop for EOL versions of software it's just the way the ecosystem works

It will really solve a lot of headaches for contributors like me, I was writing tests for my PR and maintaining so many different versions is just untennable, I spent 2 hours on backporting the test to work on laravel 5 (of course only after the PR was submitted because without CI, we only test the latest version locally)

winkelco · 2023-09-08T02:15:53Z

@patrickbrouwers @Tofandel any idea to use the batchable trait only if the laravel version is >= 8?

Tofandel · 2023-09-08T07:23:07Z

Yes, copy the original class to an abstract class with a different name

Then, in the original file do a trait_exists and declare the class extending the abstract either with the trait or without

Though my recommendation is still to simply drop support of laravel < 8 so legacy versions don't get those updates and will stay on the last release until they upgrade

winkelco · 2023-09-08T14:16:56Z

@patrickbrouwers Is it very complicated to launch a version 4 simply with this feature and only admitting Laravel 8+?

patrickbrouwers · 2023-10-26T07:32:50Z

Currently don't have time for it. Unfortunately still too many people using old Laravel versions, so I don't want to carelessly drop all older versions, but have a plan first. I prefer to include a feature like this thought through and have a plan what to do with old queuing features (which I prefer to replace eventually anyway)

[FIX] Temporary file creation on multi-server setups

stale · 2023-12-30T01:46:16Z

This bug report has been automatically closed because it has not had recent activity. If this is still an active bug, please comment to reopen. Thank you for your contributions.

joelharkes · 2024-01-21T19:24:03Z

ons, so I don't want to carelessly drop all older versions, but have a plan first. I prefer to include a feature like this thought throug

Why motivate people to use insecure software? I honestly don't understand. If they need the latest Laravel Excel feature give them the benefit that they also must update their Laravel project and as bonus benefit have Laravel security updates again..

https://endoflife.date/laravel

patrickbrouwers · 2024-01-22T11:52:46Z

Why motivate people to use insecure software? I honestly don't understand. If they need the latest Laravel Excel feature give them the benefit that they also must update their Laravel project and as bonus benefit have Laravel security updates again..

https://endoflife.date/laravel

Nowhere I say I motivate/encourage/endorse using older Laravel versions. I'm only basing myself on amount of packagist installs and opened issues.

I'll also paste my reply to another tread here to further explain.

Unfortunately the majority of issues opened here are of people using older Laravel versions with the latest version of this package.
Features like queue batches are great, but I want to think it through carefully before releasing it to a lot of people. Having a package with a lot of users comes with a responsibility and less freedom. The last time I dropped older stuff I got huge backlash and way too many complaints. Not fun I can tell you. Therefore I decided to do a major release in the future which will - from that time on - only support the actively supported Laravel and PHP versions. (Which will be documented to avoid surprises) I currently don't have time to finish the last bits of the planned 4.0 release, so any version dropping will also have to wait until then. I'm sorry if that's inconvenient to some, but that's also open source 😅

For now I'll lock this conversation, not because I don't value opinions on this, just because further discussions about this are taking up the little time I have for open source nowadays (I get paid to do a lot of other cool stuff, and I'm happy I get to spend time on this at all lately which is for sure not a given) and I prefer to spent it on working towards the 4.0 release.

winkelco added 3 commits August 3, 2023 00:48

ShouldBatch Concern

68b0193

Import with batches

b6c2028

Export with batches

5383e48

patrickbrouwers requested changes Aug 3, 2023

View reviewed changes

patrickbrouwers reviewed Aug 3, 2023

View reviewed changes

patrickbrouwers added the postponed label Oct 26, 2023

winkelco added 2 commits October 29, 2023 08:28

[FIX] Temporary file creation on multi-server setups

0595d0a

Merge pull request #1 from winkelco/winkelco-patch-1

3a1aec7

[FIX] Temporary file creation on multi-server setups

stale bot added the stale label Dec 28, 2023

stale bot closed this Dec 30, 2023

patrickbrouwers reopened this Dec 30, 2023

stale bot removed the stale label Dec 30, 2023

SpartnerNL locked as resolved and limited conversation to collaborators Jan 22, 2024

SpartnerNL unlocked this conversation Jan 22, 2024

SpartnerNL locked as resolved and limited conversation to collaborators Jan 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow queue jobs with batches #3974

Allow queue jobs with batches #3974

winkelco commented Aug 3, 2023 •

edited

patrickbrouwers left a comment

patrickbrouwers Aug 3, 2023

patrickbrouwers Aug 3, 2023

winkelco Aug 25, 2023

winkelco Aug 25, 2023

patrickbrouwers Aug 3, 2023

winkelco Aug 25, 2023

joelharkes Jan 21, 2024

patrickbrouwers Jan 21, 2024

Tofandel Jan 22, 2024

patrickbrouwers Jan 22, 2024

winkelco commented Aug 25, 2023

Tofandel commented Sep 8, 2023 •

edited

winkelco commented Sep 8, 2023 •

edited

Tofandel commented Sep 8, 2023 •

edited

winkelco commented Sep 8, 2023

patrickbrouwers commented Oct 26, 2023 •

edited

stale bot commented Dec 30, 2023

joelharkes commented Jan 21, 2024

patrickbrouwers commented Jan 22, 2024 •

edited

Allow queue jobs with batches #3974

Are you sure you want to change the base?

Allow queue jobs with batches #3974

Conversation

winkelco commented Aug 3, 2023 • edited

patrickbrouwers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winkelco commented Aug 25, 2023

Tofandel commented Sep 8, 2023 • edited

winkelco commented Sep 8, 2023 • edited

Tofandel commented Sep 8, 2023 • edited

winkelco commented Sep 8, 2023

patrickbrouwers commented Oct 26, 2023 • edited

stale bot commented Dec 30, 2023

joelharkes commented Jan 21, 2024

patrickbrouwers commented Jan 22, 2024 • edited

winkelco commented Aug 3, 2023 •

edited

Tofandel commented Sep 8, 2023 •

edited

winkelco commented Sep 8, 2023 •

edited

Tofandel commented Sep 8, 2023 •

edited

patrickbrouwers commented Oct 26, 2023 •

edited

patrickbrouwers commented Jan 22, 2024 •

edited