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 Cloud Region #1134

Merged
merged 5 commits into from
May 9, 2023
Merged

Support EU Cloud Region #1134

merged 5 commits into from
May 9, 2023

Conversation

cea2aj
Copy link
Member

@cea2aj cea2aj commented May 4, 2023

Add a cloud region config option to the global_config to allow either 'us' (default), or 'eu' to be set as the cloud region.

The cloud region determines where the search assets and ytag are sourced from.

J=BACK-2277
TEST=manual, auto

Using the test site, set the cloudRegion to "eu" and confirmed the assets are loaded from the EU CDN and executed searches against the EU search endpoints

Add a cloud region config option to the global_config to allow
either 'us' (default), or 'eu' to be set as the cloud region.

The cloud region determines where the search assets and ytag
are sourced from.

J=BACK-2277
TEST=manual

Using the test site, set the cloudRegion to eu and confirmed
the assets are loaded from the eu cdn. Currently there is a
cors error, but I will retest them once Aether fixes it.
@coveralls
Copy link

coveralls commented May 4, 2023

Coverage Status

Coverage: 8.737% (-0.006%) from 8.743% when pulling 192cd6d on dev/eu-cloud-region into 17d560e on develop.

Copy link
Contributor

@EmilyZhang777 EmilyZhang777 left a comment

Choose a reason for hiding this comment

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

Only some nit.

hbshelpers/sdkAssetUrl.js Outdated Show resolved Hide resolved
test-site/config/global_config.json Outdated Show resolved Hide resolved
Copy link
Member

@benmcginnis benmcginnis left a comment

Choose a reason for hiding this comment

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

LGTM, just had one question but it can be handled later if there is a change we have to make.

@@ -24,8 +24,12 @@
<link rel="dns-prefetch" href="//dynl.mktgcdn.com">
<link rel="dns-prefetch" href="//dynm.mktgcdn.com">
Copy link
Member

Choose a reason for hiding this comment

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

do we know if these domains will be different @EmilyZhang777 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure. Based on this conversation, seems like eu is going to another bucket?

Copy link
Contributor

@EmilyZhang777 EmilyZhang777 left a comment

Choose a reason for hiding this comment

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

Only a small comment that we can address later. Otherwise, lgtm!

@@ -24,8 +24,12 @@
<link rel="dns-prefetch" href="//dynl.mktgcdn.com">
<link rel="dns-prefetch" href="//dynm.mktgcdn.com">
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure. Based on this conversation, seems like eu is going to another bucket?

@cea2aj cea2aj merged commit eb9515b into develop May 9, 2023
14 of 15 checks passed
@cea2aj cea2aj deleted the dev/eu-cloud-region branch May 9, 2023 17:48
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.

None yet

4 participants