Skip to content

Slack: Move run-creation notifications from Lambda to web app - #60

Merged
wasimxyz merged 1 commit into
stagingfrom
wa/refactor-slack-notifications
May 8, 2026
Merged

Slack: Move run-creation notifications from Lambda to web app#60
wasimxyz merged 1 commit into
stagingfrom
wa/refactor-slack-notifications

Conversation

@wasimxyz

@wasimxyz wasimxyz commented May 8, 2026

Copy link
Copy Markdown
Member

Summary

Slack notifications previously fired once per file processed by the Lambda — producing multiple "Finished preprocessing run X" messages per run, which contradicted the wording. They now fire once per newly-created run, from the web app's POST /api/v1/instruments/:instrumentId/runs endpoint, gated on the onConflictDoNothing upsert actually inserting a row.

Changes

  • New web-app/lib/slack.ts (sendSlackMessage) — reads SLACK_WEBHOOK_URL, no-ops when unset, swallows errors so a Slack outage cannot fail the API request.
  • POST .../runs posts one Slack message per isNew insert, using instruments.displayName and new URL(request.url).origin for the deep link.
  • Lambda no longer imports/calls Slack: slack.send_message, INSTRUMENT_ID_TO_NAME_MAP, and get_cloudwatch_logs_url are gone from handler.py. Per-instrument process_file functions now return None (was str) and DATA_HUB_WEB_URL / lambda/src/data_hub_lambda/constants.py are deleted.
  • data_hub_shared.slack and the SLACK_WEBHOOK_URL field on data_hub_shared.config are removed; SlackWebhookUrl is dropped from infra/template.yaml, the Makefile, deploy-lambda.yml, and the infra/.env.* / lambda/.env.example files.
  • New integration test asserts a duplicate (instrument_id, run_id) POST captures exactly one Slack message. The capture infra is an in-process HTTP server in tests/integration/global-setup.ts exposed via getCapturedSlackMessages() / clearCapturedSlackMessages().
  • Docs updated across lambda.md, architecture.md, shared-library.md, getting-started.md, ci-and-deployment.md, adding-an-instrument.md, and the root README.md.

Breaking changes

  • SLACK_WEBHOOK_URL must be configured in Vercel per environment. It can be removed from the GitHub staging / production environment secrets and from infra/.env.staging / infra/.env.production — leaving the latter in place will fail sam deploy because the SAM parameter no longer exists.
  • The data_hub_shared.slack module and config.SLACK_WEBHOOK_URL are gone; any external consumer would need to migrate (none exist in this repo).
  • Per-instrument process_file return type changed from str to None.

Driveby changes

  • Removed dead result_url assertions from lambda/tests/epson_v700_scanner/test_process_file.py.
  • Updated the "adding an instrument" guide to reflect the new process_file signature.

Testing

  • make check-all passes locally
  • Integration test in web-app/tests/integration/instrument-runs.test.ts passes (duplicate POST → single Slack message)
  • Lambda integration tests still pass with the mock_slack fixture removed
  • After deploy: trigger a new run end-to-end and confirm a single Slack message lands; trigger a second file for the same run and confirm no second message
  • Confirm sam deploy succeeds after removing SlackWebhookUrl from infra/.env.staging / infra/.env.production

The Lambda previously called slack.send_message once per file processed,
which produced multiple "Finished preprocessing run X" messages per run
even though the wording implied one. Notifications now fire from
POST /api/v1/instruments/:instrumentId/runs when the upsert actually
inserts a new row, so each run triggers exactly one Slack message
regardless of how many files arrive (lambda or watcher).

Adds web-app/lib/slack.ts (no-op when SLACK_WEBHOOK_URL is unset,
swallows errors so a Slack outage cannot fail the API request) and an
integration test using an in-process capture server. Removes
data_hub_shared.slack, the SLACK_WEBHOOK_URL config field, the
SlackWebhookUrl SAM parameter / Makefile / deploy-lambda.yml plumbing,
the dead result_url return path from each per-instrument process_file,
and the now-unused DATA_HUB_WEB_URL constant. Operators must set
SLACK_WEBHOOK_URL in the Vercel environment (it is no longer needed in
GitHub environment secrets or infra/.env.*).

Co-authored-by: Cursor <cursoragent@cursor.com>
@wasimxyz wasimxyz self-assigned this May 8, 2026
@vercel

vercel Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
data-hub Ready Ready Preview, Comment May 8, 2026 0:01am

Request Review

@wasimxyz
wasimxyz merged commit f079990 into staging May 8, 2026
6 checks passed
@wasimxyz
wasimxyz deleted the wa/refactor-slack-notifications branch May 8, 2026 00:05
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.

1 participant