Skip to content

Notification infrastructure and print notifier#6

Merged
ethanholz merged 14 commits into
omsf:mainfrom
dwhswenson:print-notifier
Jan 22, 2026
Merged

Notification infrastructure and print notifier#6
ethanholz merged 14 commits into
omsf:mainfrom
dwhswenson:print-notifier

Conversation

@dwhswenson
Copy link
Copy Markdown
Member

This PR adds some of the notification infrastructure (some source already in src/cloud_cron/notifications/base.py) as well as a first concrete implementation in the PrintNotificationHandler. This example implementation just prints the example notification (making it visible in CloudWatch logs for the print notifier lambda function).

This also migrates SNS topics to FIFO SNS topics, which is necessary to connect to a FIFO SQS queue.

@dwhswenson dwhswenson marked this pull request as draft January 18, 2026 18:33
@dwhswenson dwhswenson marked this pull request as ready for review January 20, 2026 18:32
@dwhswenson dwhswenson requested a review from Copilot January 20, 2026 18:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces notification infrastructure for cloud-cron, adding a reusable notification handler framework and a concrete print-based implementation for testing. It also migrates SNS topics to FIFO to enable integration with FIFO SQS queues, which is required for message deduplication.

Changes:

  • Adds PrintNotificationHandler that extends the base NotificationHandler to print rendered Jinja2 templates to CloudWatch logs
  • Migrates SNS topics to FIFO by automatically appending .fifo suffix and adding MessageGroupId support to SNS publish operations
  • Introduces reusable Terraform modules: notification-plumbing for SNS→SQS→Lambda wiring and print-notification for the print handler deployment

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/cloud_cron/notifications/print_handler.py New handler that prints rendered templates for testing notification pipeline
src/cloud_cron/notifications/__init__.py Exports the new PrintNotificationHandler class
src/cloud_cron/lambda_task.py Adds load_sns_message_group_id() function and includes MessageGroupId in SNS publish calls
tests/notifications/test_print_handler.py Tests for PrintNotificationHandler's template rendering and output
tests/test_lambda_task.py Tests for load_sns_message_group_id() and updated SNS publish assertions
modules/sns-topics/main.tf Converts SNS topics to FIFO by auto-appending .fifo suffix and enabling content-based deduplication
modules/notification-plumbing/main.tf New reusable module for SNS→SQS FIFO→Lambda event source mapping with IAM policy output
modules/notification-plumbing/variables.tf Input validation for FIFO topic ARN and queue name
modules/notification-plumbing/outputs.tf Exports queue ARNs, subscription details, and Lambda IAM policy JSON
modules/notification-plumbing/README.md Documentation for notification plumbing module usage
modules/print-notification/main.tf Lambda function, IAM roles/policies, and plumbing integration for print handler
modules/print-notification/variables.tf Configuration for Lambda image, template file, and queue settings
modules/print-notification/outputs.tf Exports Lambda ARN, queue details, and subscription ARN
modules/print-notification/README.md Documentation for print notification module usage
examples/basic/main.tf Integrates print notification module with example SNS topic
examples/basic/variables.tf Adds print_repository_name variable for container repository
examples/basic/templates/print.txt Jinja2 template for print notification output
examples/basic/print-notifier/handler.py Lambda handler entry point using PrintNotificationHandler
examples/basic/print-notifier/Dockerfile Container image definition for print notification Lambda
PLAN.md Updates Phase 4 tasks to reflect completed notification plumbing work
IDEA.md Updates notification channel description to reflect per-channel containers

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/print-notification/main.tf Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 21, 2026

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@dwhswenson dwhswenson requested a review from ethanholz January 21, 2026 22:59
@ethanholz
Copy link
Copy Markdown
Contributor

This all looks correct but I can't really tell without testing this on real infrastructure. Have you run any tofu plan/tofu apply to this works as you expect?

@dwhswenson
Copy link
Copy Markdown
Member Author

This all looks correct but I can't really tell without testing this on real infrastructure. Have you run any tofu plan/tofu apply to this works as you expect?

Getting this once an hour in CloudWatch logs on AWS (since.... too long? note -- locally managed state for now; in the future we'll add tests that this at least deploys correctly)

image

I'll call that passing!

@ethanholz
Copy link
Copy Markdown
Contributor

Looks good! Let's merge that in then!

@ethanholz ethanholz merged commit c614c05 into omsf:main Jan 22, 2026
3 checks passed
@dwhswenson dwhswenson deleted the print-notifier branch January 22, 2026 15:10
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