GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat: geolocation setter in sendgrid-python for GDPR compliance by manisha1997 · Pull Request #1073 · sendgrid/sendgrid-python

Conversation

@manisha1997

@manisha1997 manisha1997 commented Nov 14, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

feat!: Geolocation setter for GDPR compliance

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
manisha1997 added 2 commits November 14, 2023 11:18
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
30b1ca6
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
1e1299d
@manisha1997
manisha1997 force-pushed the 7.0.0-rc branch from 1dc2b2d to 1e1299d Compare November 14, 2023 08:15
shrutiburman
shrutiburman reviewed Nov 14, 2023
View reviewed changes
Comment thread test/unit/test_sendgrid.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/unit/test_sendgrid.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sendgrid/base_interface.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
shrutiburman
shrutiburman reviewed Nov 14, 2023
View reviewed changes
Comment thread sendgrid/base_interface.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
manisha1997 added 3 commits November 14, 2023 15:11
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
9d433a6
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
bb9535f
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
b81337d
@manisha1997
manisha1997 force-pushed the 7.0.0-rc branch from 9c2ff65 to b81337d Compare November 14, 2023 10:21
manisha1997 added 5 commits November 14, 2023 16:28
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
7e56049
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
e7b9578
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
3bd6d6f
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
cf2cc5d
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
4019024
shrutiburman
shrutiburman reviewed Nov 14, 2023
View reviewed changes
Comment thread examples/dataresidency/set_region.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
shrutiburman
shrutiburman reviewed Nov 14, 2023
View reviewed changes
Comment thread sendgrid/sendgrid.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@shrutiburman
shrutiburman requested review from gladysmae08 and tbischel November 15, 2023 04:36
@manisha1997
manisha1997 force-pushed the 7.0.0-rc branch from 2d070ea to 4780178 Compare November 16, 2023 07:57
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
6ceb524
@manisha1997
manisha1997 force-pushed the 7.0.0-rc branch from 4780178 to 6ceb524 Compare November 16, 2023 07:59
manisha1997 added 2 commits November 16, 2023 13:47
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
2a9930f
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
5885985
shrutiburman
shrutiburman approved these changes Nov 17, 2023
View reviewed changes

@shrutiburman shrutiburman left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@manisha1997

manisha1997 commented Nov 17, 2023

Copy link
Copy Markdown
Contributor Author

@tbischel, What will be the precedence of the host and data_residency.
In case, the user uses both set_host and set_data_residency, we are setting host based on the latest input.

sendgrid-python/test/unit/test_sendgrid.py

Line 19 in 639f14e

def test_host_with_host_first_eu_region_second(self):
,

sendgrid-python/test/unit/test_sendgrid.py

Line 25 in 639f14e

def test_host_with_eu_first_host_second(self):

Can you please clarify if this is correct precedence?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gladysmae08
gladysmae08 reviewed Nov 17, 2023
View reviewed changes

@gladysmae08 gladysmae08 left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

CR

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread sendgrid/base_interface.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sendgrid/base_interface.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sendgrid/base_interface.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sendgrid/base_interface.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
3dc77d8
@manisha1997
manisha1997 force-pushed the 7.0.0-rc branch from 639f14e to 3dc77d8 Compare November 20, 2023 09:10
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
da3f03c
tbischel
tbischel reviewed Nov 21, 2023
View reviewed changes
Comment thread examples/dataresidency/set_region.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
tbischel
tbischel reviewed Nov 21, 2023
View reviewed changes
Comment thread sendgrid/base_interface.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@tbischel

tbischel commented Nov 21, 2023

Copy link
Copy Markdown

regarding precedence:
I think if they explicitly call the setters, it makes sense to respect the latest called. I think if they pass region and host in the constructor at the same time, we should throw an error if they conflict.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@manisha1997

manisha1997 commented Nov 21, 2023

Copy link
Copy Markdown
Contributor Author

Makes sense Tyler, thanks.
Will introduce the changes accordingly.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
manisha1997 added 4 commits November 22, 2023 14:01
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
f86747b
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
0a95877
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
41312f4
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
9f73f25
@manisha1997

manisha1997 commented Nov 22, 2023

Copy link
Copy Markdown
Contributor Author

@tbischel, we decided to remove region and only have a data residency setter.
We removed the set_host as we don't want to expose the function right now #1073 (comment).
Can you please review the changes
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
7eb07f7
tbischel
tbischel reviewed Nov 22, 2023
View reviewed changes
Comment thread examples/dataresidency/set_region.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@tbischel

tbischel commented Nov 22, 2023

Copy link
Copy Markdown

minor nit with the print placement, but other than that LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
d760424
@tiwarishubham635
tiwarishubham635 self-requested a review November 23, 2023 09:33
@tiwarishubham635

tiwarishubham635 commented Nov 23, 2023

Copy link
Copy Markdown
Contributor

LGTM!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
tiwarishubham635
tiwarishubham635 approved these changes Nov 23, 2023
View reviewed changes
@manisha1997
feat: geolocation setter in sendgrid-python for GDPR compliance
7fc733d
tbischel
tbischel approved these changes Nov 23, 2023
View reviewed changes

@tbischel tbischel left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@shrutiburman
shrutiburman merged commit b5afcca into main Nov 23, 2023
@shrutiburman
shrutiburman deleted the 7.0.0-rc branch November 23, 2023 18:59
This was referenced Nov 21, 2024
Support EU region endpoints sklarsa/django-sendgrid-v5#129
Closed
Support EU region endpoint sklarsa/django-sendgrid-v5#131
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbischel tbischel tbischel approved these changes
@shrutiburman shrutiburman shrutiburman approved these changes
+2 more reviewers
@gladysmae08 gladysmae08 gladysmae08 left review comments
@tiwarishubham635 tiwarishubham635 tiwarishubham635 approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@manisha1997 @tbischel @tiwarishubham635 @gladysmae08 @shrutiburman
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.