Skip to content

Commit

Permalink
r laravel 8
Browse files Browse the repository at this point in the history
  • Loading branch information
@dusterio
dusterio committed Oct 8, 2020
1 parent e6c6cba commit eadfb94
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
],
"require": {
"php": ">=5.5.0",
"illuminate/support": "5.*|^6.0|^7.0",
"illuminate/queue": "5.*|^6.0|^7.0",
"illuminate/bus": "5.*|^6.0|^7.0",
"illuminate/support": "5.*|^6.0|^7.0|^8.0",
"illuminate/queue": "5.*|^6.0|^7.0|^8.0",
"illuminate/bus": "5.*|^6.0|^7.0|^8.0",
"aws/aws-sdk-php": "~3.0"
},
"require-dev": {
Expand Down

6 comments on commit eadfb94

@nickturrietta
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dusterio does the Sqs/Queue.php file need to be updated as well for Laravel 8?

as in this pull request from the @jason-klein:
https://github.com/dusterio/laravel-plain-sqs/pull/47/files

@dusterio
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nickturrietta it's already updated and tagged :) composer update should work

@jason-klein
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dusterio Your package does install in Laravel 8, but is not processing Jobs correctly due to the issue @nickturrietta mentioned above.

Line 75 in the Queue.php file also needs to be updated to reflect the current Laravel Version. Laravel returns a Container Version that matches the current Laravel Version. The contents of $this->container->version() for Laravel 8.9.0 are currently Container Version [8.9.0].

See my suggested edit in PR #47: src/Sqs/Queue.php

Thanks for continuing to maintain this package! 😄

@dusterio
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jason-klein ops, you are right! interestingly it worked fine for me - I'm using this package at one of my products

I'm re-opening and merging your PR now, thanks!

@jason-klein
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dusterio Thanks! For future reference, here's the error we were receiving when we attempted to process a Job from a Plain SQS message without the update I made to Queue.php.

Argument 3 passed to Illuminate\Queue\Jobs\SqsJob::__construct() must be of the type array, string given, called in vendor/dusterio/laravel-plain-sqs/src/Sqs/Queue.php on line 80 {"exception":"[object] (TypeError(code: 0): Argument 3 passed to Illuminate\\Queue\\Jobs\\SqsJob::__construct() must be of the type array, string given, called in vendor/dusterio/laravel-plain-sqs/src/Sqs/Queue.php on line 80 at vendor/laravel/framework/src/Illuminate/Queue/Jobs/SqsJob.php:35)

We've been successfully running PR #47 in production since Sept 10, 2020. We process thousands of Jobs from SQS per day, including both standard Laravel Jobs from Laravel SQS messages and other notifications from Plain SQS messages (e.g. S3 notifications to SQS) so users shouldn't have any issues with our PR.

@nickturrietta
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks @jason-klein! And @dusterio for this most-excellent package! 😃

Please sign in to comment.