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

TwilioPhoneProvider optimizations #2034

Merged
merged 4 commits into from
Jun 8, 2023
Merged

Conversation

Konstantinov-Innokentii
Copy link
Member

@Konstantinov-Innokentii Konstantinov-Innokentii commented May 26, 2023

What this PR does

This PR does three improvements in twilio_phone_provider:

  1. Speed-up query which calculates amount of phone_calls/sms left.
  2. Remove code which was needed only for backward compatibility during the release of PhoneProvider refactoring and improves logging for handling status/gather updates.
  3. Add db_index to twilio_sid. We are doing lot of lookups by sid and with increasing amount of data it became resource consuming.

@Konstantinov-Innokentii Konstantinov-Innokentii requested a review from a team May 26, 2023 10:46
@Konstantinov-Innokentii Konstantinov-Innokentii added pr:no changelog pr:no public docs Added to a PR that does not require public documentation updates labels May 26, 2023
@Konstantinov-Innokentii Konstantinov-Innokentii changed the title Fix call_left query TwilioPhoneProvider optimizations Jun 1, 2023
status_code = TwilioCallStatuses.DETERMINANT.get(call_status)

if status_code is None:
logger.warning(
Copy link
Contributor

Choose a reason for hiding this comment

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

how normal is receiving unexpected status? This code won't alert us if we start receiving unexpected statuses

Copy link
Member Author

Choose a reason for hiding this comment

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

It's ok, status is only needed for better timeline. The thing about checking status always was in twilio part, so I decided to at least log it

@@ -61,12 +61,10 @@ def _calculate_phone_notifications_left(self, user):
day_start = now.replace(hour=0, minute=0, second=0, microsecond=0)
calls_today = PhoneCallRecord.objects.filter(
created_at__gte=day_start,
represents_alert_group__channel__organization=self.organization,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

we remove it because user can be a member of single organization and organization filter is excess, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

right

@Konstantinov-Innokentii Konstantinov-Innokentii added this pull request to the merge queue Jun 8, 2023
Merged via the queue into dev with commit 306f842 Jun 8, 2023
@Konstantinov-Innokentii Konstantinov-Innokentii deleted the fix_call_left_query branch June 8, 2023 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants