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

Fix: Telegram and issue reporting improvements #59

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

Seth-Schmidt
Copy link
Contributor

Fixes ##58

Checklist

  • My branch is up-to-date with upstream/develop branch.
  • Everything works and tested for Python 3.8.0 and above.
  • I ran pre-commit checks against my changes.
  • I've written tests against my changes and all the current present tests are passing.

Current behaviour

The implementation of the telegram issue reporting service is currently inconsistent with established internal patterns, and the handling of httpx clients used is incorrect/can be improved.

New expected behaviour

I have separated the telegram reporting logic from the reporting service's in utils/callback_helpers.py. Two new functions called send_telegram_notification_sync and send_telegram_notification_async have been created. These will handle sending the notification message to the correct telegram service endpoint depending on the issue that was raised.

An additional httpx client has been added to the core snapshotter services that opens a connection pool against the telegram reporting service endpoint. The telegram and reporting service notifications will now each have their own callback helpers and clients.

The call to sleep in system_event_detector after _init_check_and_report() has completed has also been removed.

Change logs

Changed

  • utils/callback_helpers.py
    • created separate helpers for telegram notifications
    • reverted changes to internal reporting helpers
  • system_event_detector.py
    • updated issue reporting for new callback helpers
    • _send_telegram_epoch_processing_notification() created to remove duplicate code
    • added self._telegram_httpx_client
    • removed sleep after simulation submissions are complete
  • processor_distributor.py
    • updated issue reporting for new callback helpers
    • _send_failure_notifications() created to handle internal/telegram reporting and remove duplicate code
    • added self._telegram_httpx_client
  • utils/generic_worker.py
    • updated issue reporting for new callback helpers
    • _send_failure_notifications() created to handle internal/telegram reporting and remove duplicate code
    • added self._telegram_httpx_client
  • utils/snapshot_worker.py
    • updated issue reporting for new callback helpers
    • uses inherited notification method
  • utils/models/
    • updated telegram message models and moved them to the message_models file
    • removed deprecated models due to reporting changes

Removed

  • SnapshotterReportData from utils/models/data_models.py

Deployment Instructions

No changes to deployment.

@Seth-Schmidt Seth-Schmidt changed the base branch from main to dockerify September 12, 2024 01:14
@Seth-Schmidt Seth-Schmidt self-assigned this Sep 12, 2024
@SwaroopH SwaroopH changed the base branch from dockerify to main September 13, 2024 10:02
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