Added SkipsAfterImportJob Concern

[mahdi29]

trafficstars

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? ☑️ Excel file in remote (s3) location gets deleted before every queue chunk is processed while using ShouldQueueWithoutChain concern as it is not possible to be sure a specific job executes last. Therefore adding SkipsAfterImport job concern completely eliminates the AfterImportJob, thus file is not deleted. (solving #3560) 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]

As mentioned in #3560 I would be okay with a SkipsCleanup. Skipping the entire after import job, also removes the benefit of being able to use the event system.

[kchoppin]

Job batching seems to be the answer, but this is only available in Laravel 8+ and requires a migration. Then delaycleanup, ShouldQueueWithoutChain etc. is not needed as AfterImport can be set to run after all jobs are processed.

eg having something like this in the ChunkReader should work;

        if ($import instanceof ShouldBatch) {
            return Bus::batch($jobs)
                ->then(function(Batch $batch) use ($afterImportJob, $queue) {
                    dispatch($afterImportJob)->onQueue($queue);
                })->onQueue($queue)->dispatch();
        }

Happy to help with a PR, just not sure if the requirements are too high?

PS: I've not actually experienced the issue of the file being deleted when running jobs without a chain, it seems to run AfterImportJob at the time specified. But it can be a pain waiting for a fixed amount of time.

[patrickbrouwers]

Besides that batches indeed aren't supported by <8, it also has its own set of issues that I need to carefully consider when adding it to the package; as I've mentioned in a few other tickets as well, that's something I will have a closer look at when going for the 4.0 release.

[stale[bot]]

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.