-
Notifications
You must be signed in to change notification settings - Fork 10
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
JOBS-10957: Fix SQS error handling / general refactoring #43
base: master
Are you sure you want to change the base?
Conversation
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.
Nice one!
It's a fundamental change - how can we test this and be sure it's working properly?
src/ServiceBusSQSChannel.php
Outdated
|
||
$eventName = $params['events'][0]; | ||
|
||
Log::info("{$eventName} sent to bus queue", [ |
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.
is there an opportunity for this bit to respect the dontreport config option too? :D
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.
scratch this wont make sense.. we need another config to determine the loglevel or quiet this donw altogether.. dontreport is used for something else entirely
36f774f
to
d6a21d3
Compare
|
@datashaman, the builds are failing on GitHub, which is concerning. I ran the tests locally with these changes, and they passed. Can you check if the PR is applicable for merging and proceed if so? |
@otanim the builds are failing because of a change in your code. look at the failing build. fix the code. |
Ticket Number:
https://oneafricamedia.atlassian.net/browse/JOBS-10957