-
Notifications
You must be signed in to change notification settings - Fork 716
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 Proxies #409
Comments
Go for it!! If this is something that you need, someone else might need it to. Please make a PR of your change, so long as it is documented, configurable, testable, and passes our build and testing - we will get it merged in. |
I guess this would need ProxyHandler added in python_http_client first or at the same time, right? Would the intention be to add proxies kwargs when instatiating SendGridAPIClient ? And then pass the kwargs through to python_http_client Client? |
Hi @campbellmc, Yes, that is correct. Thanks! |
@thinkingserious May I ask why is sendgrid-python/sendgrid/sendgrid.py Lines 35 to 42 in 19bb394
|
Looking through the code I didn't find support for proxy-based integrations. Are you still taking PRs for this open request? |
As a side-note - we worked around this by calling requests.post independently.
Alas, time is short to make a PR but this work-around was shorter so you can guess which won. |
|
Thanks @campbellmc, thats a good work-around! @splashx Thats a good suggestion, although we don't want to proxy all of our daemon's network requests through the specific proxy we are using because of network costs. These proxy requests are for running it through a secure tokenizer for email addresses and other PII to be revealed to sendgrid. |
as an example of how to build a request with proxies:
the above code might not be good in production. |
I have to use sendgrid to send newsletter mails. Being under a institute proxy, I have to SSH into a AWS server and then pull my script+data there and then send my mails from there.
Now I understand this feature isn't that a big improvement, but this would really help me. I want to make this fix in this package.
What are your opinions?
The text was updated successfully, but these errors were encountered: