Skip to content
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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

otanim
Copy link
Collaborator

@otanim otanim commented Oct 30, 2024

@otanim otanim changed the title Fix sqs error handling Fix SQS error handling / general refactoring Oct 30, 2024
Copy link

@patrickarnold79 patrickarnold79 left a 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?


$eventName = $params['events'][0];

Log::info("{$eventName} sent to bus queue", [
Copy link
Collaborator

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

Copy link
Collaborator

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

src/ServiceBusSQSChannel.php Outdated Show resolved Hide resolved
@otanim otanim force-pushed the fix-sqs-error-handling branch from 36f774f to d6a21d3 Compare October 30, 2024 21:39
@otanim
Copy link
Collaborator Author

otanim commented Nov 4, 2024

please build

@otanim
Copy link
Collaborator Author

otanim commented Nov 4, 2024

@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 otanim changed the title Fix SQS error handling / general refactoring JOBS-10957: Fix SQS error handling / general refactoring Nov 6, 2024
@datashaman
Copy link
Contributor

@otanim the builds are failing because of a change in your code. look at the failing build. fix the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants