feat: Add data residency for eu and global regions #1390

shrutiburman · 2023-11-20T08:32:06Z

Fixes DII-1230

This PR:

Adds a setter setDataResidency() which allows the users to switch to EU sub region, reidrecting their requests to api.eu.sendgrid.com
Upgrades the testing pipeline to node v10+ (eslint and typescript do not work well on <10 versions)
Upgrades typescript, which was causing the pipleine failure for very long

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.

manisha1997

LGTM

gladysmae08

CR

packages/client/src/client.spec.js

tiwarishubham635

LGTM!

docs/use-cases/data-residency-set-hostname.md

tbischel · 2023-11-28T17:22:02Z

docs/use-cases/data-residency-set-hostname.md

+
+## Limitations
+
+1. Setting the API Key (via `client.setApiKey()`) or Twilio Authentication (via `client.setTwilioEmailAuth()`) will override the hostname to default value. Use the setter call after this set-up.


is this something we should figure out a better path for? This is not an ideal scenario.

@tbischel @gladysmae08
Hi, I hope my PR offers a solution to this issue. Please take a look and I would appreciate any feedback.

I think this limitation is no longer necessary after the changes that were just added

this won't introduce breaking changes for existing customers, right?

if customers don't have to change their code to achieve the same functionality, I think I'm good. if they do, we might want to look into a major version bump?

This wont introduce a breaking change, as the region attribute that's newly introduced has a default value, and will only get updated if the setter for data residency is called.

tbischel

I think we need to resolve this usability gap on the setApiKey and setDataResidency before we release.

gladysmae08

CR

packages/client/src/classes/client.js

gladysmae08 · 2023-11-29T17:50:50Z

packages/client/src/classes/client.js

+    // this means that region was never set before
+    if (this.sendgrid_region == '') {
+      this.setDefaultRequest('baseUrl', SENDGRID_BASE_URL);
+    }


This technically works but I think it is unnecessary. We can just always reset the baseUrl here based on the saved region

this.setDefaultRequest('baseUrl', REGION_HOST_MAP[this.sendgrid_region]);

Yup, I have set the default region as 'global', we can reset based on that now. Thanks.

gladysmae08 · 2023-11-29T17:53:22Z

docs/use-cases/data-residency-set-hostname.md

+
+## Limitations
+
+1. Setting the API Key (via `client.setApiKey()`) or Twilio Authentication (via `client.setTwilioEmailAuth()`) will override the hostname to default value. Use the setter call after this set-up.


I think this limitation is no longer necessary after the changes that were just added

mvanzoest · 2023-11-29T20:01:33Z

Hi, would it be possible to open a separate PR for the version upgrades? A number of people are awaiting #1387 in order to complete their security upgrades.

shrutiburman · 2023-11-30T05:16:00Z

I think we need to resolve this usability gap on the setApiKey and setDataResidency before we release.

We have resolved the discrepancy in the latest commits. The limitation no longer exists.

gladysmae08

LGTM

shrutiburman added 3 commits November 17, 2023 16:46

init

91b4cb6

Added tests

50a8691

Add an example readme

2f7c9ea

shrutiburman requested review from tbischel, gladysmae08, manisha1997 and tiwarishubham635 November 20, 2023 08:32

Add v12

a246759

manisha1997 approved these changes Nov 20, 2023

View reviewed changes

nit

1c79ce7

k725 mentioned this pull request Nov 21, 2023

fix(axios): updated axios version to fix critical vulnerability #1387

Open

8 tasks

gladysmae08 reviewed Nov 21, 2023

View reviewed changes

packages/client/src/client.spec.js Show resolved Hide resolved

update limitations

a21b169

tiwarishubham635 approved these changes Nov 22, 2023

View reviewed changes

tiwarishubham635 mentioned this pull request Nov 22, 2023

Deps update - snyk risks #1388

Open

8 tasks

updates to baseUrl setter

b356716

shrutiburman requested a review from gladysmae08 November 23, 2023 13:19

tbischel reviewed Nov 28, 2023

View reviewed changes

docs/use-cases/data-residency-set-hostname.md Outdated Show resolved Hide resolved

tbischel reviewed Nov 28, 2023

View reviewed changes

tbischel requested changes Nov 28, 2023

View reviewed changes

gladysmae08 reviewed Nov 29, 2023

View reviewed changes

shrutiburman requested review from tbischel and gladysmae08 November 30, 2023 05:23

review comments

8bfe5cd

shrutiburman force-pushed the add-region-setter branch from 319754e to 8bfe5cd Compare November 30, 2023 05:26

Merge branch 'main' into add-region-setter

3f90c9f

gladysmae08 approved these changes Dec 1, 2023

View reviewed changes

Merge branch 'main' into add-region-setter

d0cc9e5

tbischel approved these changes Dec 5, 2023

View reviewed changes

shrutiburman merged commit eed8ba2 into main Dec 6, 2023
13 checks passed

shrutiburman deleted the add-region-setter branch December 6, 2023 05:07

WontonSam mentioned this pull request Jul 17, 2024

[Snyk] Upgrade @sendgrid/mail from 7.7.0 to 8.1.3 WontonSam/website#28

Open

paaschdigital mentioned this pull request Sep 9, 2024

[Snyk] Upgrade @sendgrid/mail from 7.7.0 to 8.1.3 paaschdigital/notion-sdk-js#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add data residency for eu and global regions #1390

feat: Add data residency for eu and global regions #1390

shrutiburman commented Nov 20, 2023

manisha1997 left a comment

gladysmae08 left a comment

tiwarishubham635 left a comment

tbischel Nov 28, 2023

Be3751 Nov 29, 2023

gladysmae08 Nov 29, 2023

tbischel Dec 1, 2023

tbischel Dec 1, 2023

shrutiburman Dec 5, 2023

tbischel left a comment

gladysmae08 left a comment

gladysmae08 Nov 29, 2023

shrutiburman Nov 30, 2023

gladysmae08 Nov 29, 2023

mvanzoest commented Nov 29, 2023

shrutiburman commented Nov 30, 2023

gladysmae08 left a comment


		## Limitations

		1. Setting the API Key (via `client.setApiKey()`) or Twilio Authentication (via `client.setTwilioEmailAuth()`) will override the hostname to default value. Use the setter call after this set-up.

feat: Add data residency for eu and global regions #1390

feat: Add data residency for eu and global regions #1390

Conversation

shrutiburman commented Nov 20, 2023

Fixes DII-1230

Checklist

manisha1997 left a comment

Choose a reason for hiding this comment

gladysmae08 left a comment

Choose a reason for hiding this comment

tiwarishubham635 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbischel left a comment

Choose a reason for hiding this comment

gladysmae08 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvanzoest commented Nov 29, 2023

shrutiburman commented Nov 30, 2023

gladysmae08 left a comment

Choose a reason for hiding this comment