Refactor Slack message construction logic#526
Open
andrewmogan wants to merge 24 commits intodevelopfrom
Open
Conversation
Using gh api to get the workflow conclusion while the workflow is still running returns a null value. Since the Slack message workflow will (presumably) always run during a workflow, we instead have to infer the workflow status based on whether failed/cancelled jobs are present in the json summary.
…o amogan/refactor_slack_message_builder
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR improves the maintainability of the automated Slack message generation for nightly workflows. The primary motivation is that I found the present implementation difficult to extend when trying to create a new message type for "new release appeared on CVMFS", which is currently done by Cronitor.
The
gh apicall fromslack-notification.ymlhas been factored into its own script,get_workflow_summary.sh. This script outputsworkflow_summary.json, a required input toslack_message_builder.py(renamed fromslack_payload_generator.py). Rather than the message builder script take an ever-growing number of command-line arguments, additional workflow summary data, such as release name or location of pytest logs, can now be added during theAdd metadata to workflow summarystep of the Slack notification workflow.The
slack_message_builder.pyscript uses the input json summary file to pick a message building strategy. Each message strategy assembles a list ofBlocks and writes that data toslack_payload.json. TheBaseMessageStrategyserves as a template from which other strategies can build. For example, theIntegrationTestMessageStrategyadds aBlockfor the location of pytest logs.Finally, I've removed the XML parsing logic which attempts to summarize the cause of integration test failures. These messages tend to boil down to "there are errors in the log files," which is not incredibly helpful. Failure messages will now simply list the jobs and steps that failed. This also means that the
slack-notification.ymlworkflow no longer takes the junit XML directory as input. If we really want to revive this logic, it should live in the (also newly refactored)integtest_xml_parser.py.Aside from some minor formatting changes, this should preserve the present functionality. I've tested the relevant workflows in the private
#gha-slack-testschannel, which you can see in the screenshot below.