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

add support for configurable sms backend #2024

Closed

Conversation

AnkurPrabhu
Copy link
Contributor

@AnkurPrabhu AnkurPrabhu commented Mar 27, 2024

Proposed Changes

  • Have used django-sms and make a custom backend for our existing sns

Associated Issue

Merge Checklist

  • Tests added/fixed
  • Update docs in /docs
  • Linting Complete
  • Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

@AnkurPrabhu
Copy link
Contributor Author

@sainak ptal, shoud i add tests for other places as well ?

@sainak sainak changed the title adding configurable backend add support for configurable sms backend Mar 30, 2024
Pipfile Outdated
@@ -45,6 +45,7 @@ requests = "==2.31.0"
sentry-sdk = "==1.30.0"
whitenoise = "==6.5.0"
redis-om = "==0.2.1"
django-sms = "*"
Copy link
Member

Choose a reason for hiding this comment

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

pin to a specific version


class CustomSMSBackendSNS(BaseSmsBackend):
def send_messages(self, messages: List[Message]):
client = boto3.client(
Copy link
Member

Choose a reason for hiding this comment

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

initialize the client in __init__()

except Exception:
continue

client.publish(PhoneNumber=phone_number, Message=message_body)
Copy link
Member

Choose a reason for hiding this comment

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

handle exceptions

@@ -48,3 +48,5 @@
RUNSERVER_PLUS_PRINT_SQL_TRUNCATE = 100000

DISABLE_RATELIMIT = True

SMS_BACKEND = "care.utils.sms.sendSMS.CustomSMSBackendSNS"
Copy link
Member

Choose a reason for hiding this comment

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

set SMS_BACKEND = "care.utils.sms.sendSMS.CustomSMSBackendSNS" in base settings
and for local and test use console backend

@sainak
Copy link
Member

sainak commented Mar 30, 2024

@vigneshhari though the django-sms provides a standard wrapper for sms, the backend themselves have different names for settings, and some backends require client sdks to be installed
also I just noticed that django-sms has a 3BSD license

@AnkurPrabhu AnkurPrabhu requested a review from a team as a code owner April 5, 2024 20:37
@AnkurPrabhu
Copy link
Contributor Author

@sainak let me know if the changes made look good to you

@sainak
Copy link
Member

sainak commented Apr 8, 2024

Hey @AnkurPrabhu thanks for these changes, but we are planning to move the sms backends to a plugin system, once the plugin architecture becomes stable, we will create our own implementation for the sms

cc: @vigneshhari

@sainak sainak added the Hold label Apr 8, 2024
@sainak sainak added the to-be-closed PRs with no updates in the last 3 weeks will be closed label May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hold to-be-closed PRs with no updates in the last 3 weeks will be closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configurable SMS Providers
3 participants