Skip to content

refactor(integrations): hoist delivery-readiness helpers into platform/scheduler (#4164)#4187

Closed
bowale01 wants to merge 3 commits into
Tracer-Cloud:mainfrom
bowale01:refactor/hoist-delivery-readiness-to-platform
Closed

refactor(integrations): hoist delivery-readiness helpers into platform/scheduler (#4164)#4187
bowale01 wants to merge 3 commits into
Tracer-Cloud:mainfrom
bowale01:refactor/hoist-delivery-readiness-to-platform

Conversation

@bowale01

Copy link
Copy Markdown
Contributor

Refs #4164

Description

Move the cross-cutting delivery-readiness helpers from integrations/sentry/digest_delivery.py to platform/scheduler/delivery_readiness.py so no vendor owns them and both Sentry and PostHog can consume them from a neutral location.

Changes:

  • Created platform/scheduler/delivery_readiness.py with the canonical implementation
  • Converted integrations/sentry/digest_delivery.py to a thin re-export shim for backward compatibility
  • Updated integrations/sentry/digest_prerequisites.py to import from platform/
  • Updated test monkeypatch paths to point to the new canonical module

Testing

  • pytest tests/integrations/sentry/test_digest_delivery.py — all 4 tests pass
  • ruff check passes
  • No vendor-to-vendor import dependency for delivery readiness

Code Understanding and AI Usage

  • Yes, I used AI assistance (continue below)
  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Implementation approach: Moved helpers to platform/scheduler/delivery_readiness.py (sibling to credentials.py). Kept the original file as a re-export shim so any existing consumer that imports from integrations.sentry.digest_delivery still works. Updated digest_prerequisites.py to import from the new canonical location directly.


Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • My code follows the project's style guidelines and conventions

Copilot AI review requested due to automatic review settings July 21, 2026 22:36
@github-actions

Copy link
Copy Markdown
Contributor

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hoists the delivery-readiness helpers out of the Sentry-owned integrations/sentry/digest_delivery.py into a new, vendor-neutral platform/scheduler/delivery_readiness.py, then converts the original file into a thin re-export shim for backward compatibility.

  • New canonical module (platform/scheduler/delivery_readiness.py): exact logic parity with the original, plus a minor hint-text improvement that now also advertises SLACK_ACCESS_TOKEN (consistent with how resolve_slack_credentials actually resolves credentials).
  • Shim (integrations/sentry/digest_delivery.py): re-exports all five symbols from the canonical module; __all__ kept in sync and enforced by the new smoke tests in test_digest_delivery_shim.py.
  • Tests: original test file renamed to mirror the canonical module path (tests/platform/scheduler/test_delivery_readiness.py); all monkeypatch targets updated to platform.scheduler.delivery_readiness.*; new shim smoke tests added to guard against __all__ drift.

Confidence Score: 5/5

Safe to merge — pure structural refactor with no logic modifications and full backward compatibility preserved through the re-export shim.

All five delivery-readiness functions are faithfully reproduced in the new canonical module, the shim correctly delegates to them, and the previous comment about missing shim test coverage has been addressed. No logic was changed, no imports were broken, and monkeypatch targets in the test suite now point to the correct canonical location.

No files require special attention.

Important Files Changed

Filename Overview
platform/scheduler/delivery_readiness.py New canonical module containing all delivery-readiness helpers; exact logic parity with the original, plus an updated Slack hint string that now includes SLACK_ACCESS_TOKEN (accurate per credentials.py).
integrations/sentry/digest_delivery.py Converted to a thin re-export shim; all matches canonical exactly and is enforced by the new smoke tests.
integrations/sentry/digest_prerequisites.py Updated to import directly from the canonical platform module; consolidates all platform.* imports together.
tests/integrations/sentry/test_digest_delivery_shim.py New smoke test guarding against all drift between shim and canonical, and confirming all exported symbols are callable through the shim.
tests/platform/scheduler/test_delivery_readiness.py Renamed from tests/integrations/sentry/test_digest_delivery.py; all monkeypatch paths correctly updated to platform.scheduler.delivery_readiness; no coverage regressions.

Sequence Diagram

sequenceDiagram
    participant C as Consumer (e.g. digest_prerequisites.py)
    participant S as integrations/sentry/digest_delivery.py (shim)
    participant P as platform/scheduler/delivery_readiness.py (canonical)
    participant CR as platform/scheduler/credentials.py

    Note over S,P: Before this PR: logic lived only in the shim
    Note over S,P: After this PR: shim re-exports canonical

    C->>P: from platform.scheduler.delivery_readiness import delivery_provider_ready
    P->>CR: resolve_telegram_credentials / resolve_slack_credentials
    CR-->>P: credentials dict
    P-->>C: bool (ready)

    Note over S: Legacy consumers still work
    C->>S: from integrations.sentry.digest_delivery import telegram_delivery_ready
    S->>P: re-exports (same object)
    P-->>C: bool (ready)
Loading

Reviews (3): Last reviewed commit: "test(shim): assert __all__ parity betwee..." | Re-trigger Greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the scheduled digest/report delivery-readiness helpers into a vendor-neutral module under platform/scheduler/, so multiple integrations (e.g., Sentry and others) can share the same Telegram/Slack readiness logic without introducing vendor-to-vendor imports.

Changes:

  • Added platform/scheduler/delivery_readiness.py as the canonical implementation for Telegram/Slack delivery-readiness checks and setup hints.
  • Updated Sentry prerequisites and tests to import/monkeypatch the new canonical module.
  • Converted integrations/sentry/digest_delivery.py into a backward-compatibility re-export shim.

Reviewed changes

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

File Description
platform/scheduler/delivery_readiness.py New canonical, provider-neutral delivery readiness + setup hint helpers.
integrations/sentry/digest_delivery.py Re-export shim pointing to the canonical platform/ implementation.
integrations/sentry/digest_prerequisites.py Switched imports to platform.scheduler.delivery_readiness.
tests/integrations/sentry/test_digest_delivery.py Updated imports and monkeypatch paths to target the new canonical module.

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

if name == Provider.SLACK.value:
return (
"Slack is not configured for delivery. Run "
"`opensre integrations setup slack` or set SLACK_WEBHOOK_URL / SLACK_BOT_TOKEN."
Copilot AI review requested due to automatic review settings July 21, 2026 22:43
@bowale01

Copy link
Copy Markdown
Contributor Author

Addressed all review feedback:

  • Moved test to tests/platform/scheduler/test_delivery_readiness.py (mirrored path convention)
  • Added shim smoke test to guard against all drift
  • Updated Slack hint to mention SLACK_ACCESS_TOKEN

@greptile review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread tests/integrations/sentry/test_digest_delivery_shim.py Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 07:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@bowale01

Copy link
Copy Markdown
Contributor Author

@greptile review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@muddlebee muddlebee closed this Jul 22, 2026
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