DBTP-1521: Failure to post to slack can cause copilot deployment to fail #86
+219
−57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
image_builder/notify.py
Outdated
| ) | ||
| self.reference = response["ts"] | ||
| except SlackApiError as e: | ||
| logger.error(f"Slack API Error: {e.response['error']}") |
There was a problem hiding this comment.
Probably worth future-proofing this by adding a second except block for just Exception to stop the deployment failing due to any non-Slack API errors. e.g. if the SLACK_CHANNEL_ID hasn't been set or something.
image_builder/notify.py
Outdated
| from slack_sdk.models import blocks | ||
|
|
||
| from image_builder.progress import Progress | ||
| from image_builder.utils.arn_parser import ARN | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
There was a problem hiding this comment.
No longer need this as it's being created on line 30
antroy-madetech
approved these changes
Jan 10, 2025
JohnStainsby
approved these changes
Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses ticket: DBTP-1521
Added try/except block around the Slack client message methods.
logging passed into Notify class as a dependency with a default logger.
Use Logging instance to log a
SlackAPIErrortype with error messageadded Exception type error handling as per Ant’s suggestion
refactored error handling tests to not use the test class level patches for clarity
@JohnStainsby tested that the error handling was working by changing the value of the Slack parameter from AWS Parameter Store. This meant the Slack client would not have the correct value for the Slack channel which should trigger an error during the deployment process.
Expected error messages were seen in the build logs for the deploy Codebuild action. Error messages did not cause the deploy of the application image to fail this time, as expected.