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

Support EU region endpoint #131

Merged
merged 9 commits into from
Nov 26, 2024
Merged

Conversation

mikemanger
Copy link
Contributor

Adds SENDGRID_HOST_URL to configure the API's root URL. This fixes #129 by allowing people to change the default API endpoint.

I thought about using the set_sendgrid_data_residency() method added in 6.10 (sendgrid/sendgrid-python#1073) however this works with all previous releases and should "just work" if they add more regions in the future.

One downside is that to test this you need a separate subuser and API key - I've set this as TESTING_SENDGRID_EU_API_KEY.

I didn't intend to include this but an edge case to the get_payload() not always returning string is checked before doing a string replace (this is highlighted in more recent versions of mypy).

Feedback welcome.

@@ -212,7 +220,10 @@ def set_prop(attachment, prop_name, value):
filename = f"part-{uuid.uuid4().hex}{ext}"
set_prop(sg_attch, "filename", filename)
# todo: Read content if stream?
set_prop(sg_attch, "content", django_attch.get_payload().replace("\n", ""))
payload = django_attch.get_payload()
if isinstance(payload, str):
Copy link
Owner

Choose a reason for hiding this comment

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

nice catch

@sklarsa
Copy link
Owner

sklarsa commented Nov 25, 2024

@mikemanger have you tested this locally yourself? It's been a few years since I've had access to sendgrid, and the API key that I use for testing (stored as a secret in GitHub) is the only way I have to access the service without paying (I got it for free under some developer program when I first started this repo).

@mikemanger
Copy link
Contributor Author

mikemanger commented Nov 26, 2024

have you tested this locally yourself?

Yes. I've fixed the secret TESTING_SENDGRID_EU_API_KEY not getting passed to tox on my fork so it now passes both tests:

image

For some reason it is now showing as skipping for the original TestPostToSendgrid.test_post() test in this repository - did the SENDGRID_API_KEY secret get renamed or have I broken something?🐛

@sklarsa
Copy link
Owner

sklarsa commented Nov 26, 2024

I think I set it to only run on the main/master to avoid leaking the secret in PRs

@sklarsa sklarsa merged commit bcfa80e into sklarsa:master Nov 26, 2024
8 checks passed
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.

Support EU region endpoints
2 participants