Skip to content

add Slack Event notifier#81

Merged
kneeyo1 merged 3 commits intomainfrom
event-nofits
May 7, 2025
Merged

add Slack Event notifier#81
kneeyo1 merged 3 commits intomainfrom
event-nofits

Conversation

@kneeyo1
Copy link
Contributor

@kneeyo1 kneeyo1 commented May 6, 2025

This adds slack event notifs for write events.

@kneeyo1 kneeyo1 requested a review from a team as a code owner May 6, 2025 22:59
def __init__(self, key: str, url: str) -> None:
self.__notifier = slack_notifier.SlackNotifier(key, url)

def log(self, user: str, group: str, function: str, region: str) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

i'm starting to wonder if it's better/less verbose to pass a list of regions here and log a single message since we send them all to the same place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is fine the way it is, I think it would be nice to be explicit about what regions something ran in but in a loud way (maybe some test regions we don't care about)

@kneeyo1 kneeyo1 requested a review from lynnagara May 6, 2025 23:09
Copy link
Member

@lynnagara lynnagara left a comment

Choose a reason for hiding this comment

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

:shipit:

Comment on lines +145 to +146
eng_pipes_key=audit_log_data["slack"]["eng_pipes_key"],
eng_pipes_url=audit_log_data["slack"]["eng_pipes_url"],
Copy link
Member

@lynnagara lynnagara May 6, 2025

Choose a reason for hiding this comment

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

you might need to adjust the schema to allow those fields? also what doeseng_pipes mean?

@kneeyo1 kneeyo1 merged commit d7dcb57 into main May 7, 2025
11 checks passed
@kneeyo1 kneeyo1 deleted the event-nofits branch May 7, 2025 16:37
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.

3 participants