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

Adds Slack notifications about releases, upgrades to nextmv-py v0.11.1 #77

Merged
merged 5 commits into from
Sep 23, 2024

Conversation

merschformann
Copy link
Member

@merschformann merschformann commented Sep 17, 2024

Description

Added Slack notifications for improving internal transparency about ongoing releases.

Changes

  • Added optional --slack-url parameter to release workflow command
  • Added notify_slack function to send release notifications to Slack
  • Included requests>=2.26.0 in requirements.txt for HTTP requests to Slack Webhooks
  • Upgrades to nextmv-py==v0.11.1

Copy link
Member

@sebastian-quintero sebastian-quintero left a comment

Choose a reason for hiding this comment

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

Non-blocking comments

@@ -241,5 +251,21 @@ def bump_marketplace_version(
return f"v{major}.{minor}.{patch}"


def notify_slack(url: str, app: str, version: str, marketplace_version: str):
Copy link
Member

Choose a reason for hiding this comment

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

The return type hint should be None

response = requests.post(
url,
json={
"text": f"Release notification - community-apps/{app} {version} / marketplace {marketplace_version}",
Copy link
Member

Choose a reason for hiding this comment

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

I think we should either only notify for prod or log the environment that is being released in here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, totally. I somehow missed the distinction here. The parameter is optional, so, I will simply drop it in the workflow if not targeting prod. ☺️

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for noticing! 🤗

@merschformann
Copy link
Member Author

This will need a fix for nextmv-py for handling dashed parameters before merging. ☺️

@merschformann merschformann changed the title Adds Slack notifications about releases Adds Slack notifications about releases, upgrades to nextmv-py v0.11.1 Sep 23, 2024
@merschformann merschformann force-pushed the merschformann/add-slack-notifications branch from 5c0cec7 to 95f368a Compare September 23, 2024 19:38
@merschformann merschformann marked this pull request as draft September 23, 2024 19:53
@merschformann merschformann marked this pull request as ready for review September 23, 2024 19:53
@merschformann merschformann merged commit 3b634f0 into develop Sep 23, 2024
37 checks passed
@merschformann merschformann deleted the merschformann/add-slack-notifications branch September 23, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants