Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

redirect visitors based on geolocation - update host condition #18457

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

ys2843
Copy link
Contributor

@ys2843 ys2843 commented Jun 1, 2020

Description

This is a fix for #18431 . The rewrite rule is not redirecting users because of the tight host filter rule. Update the rewrite condition to not only match mxnet.apache.org, but reverse it to match all hostname except mxnet.cdn.apache.org.

According to Apache Intra team's response,

Try removing the following condition:
RewriteCond %{HTTP_HOST} mxnet.apache.org
Most of your visits go to mxnet.incubator.apache.org

Because we don't know how Apache's server is configured, remove the whole condition could possibly import looping. To safely fix this problem we can first test the updated rewrite rule.

@mxnet-bot
Copy link

Hey @ys2843 , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [miscellaneous, unix-cpu, centos-gpu, centos-cpu, windows-cpu, unix-gpu, clang, edge, windows-gpu, website, sanity]


Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

Copy link
Member

@szha szha left a comment

Choose a reason for hiding this comment

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

how are you testing it

@ys2843
Copy link
Contributor Author

ys2843 commented Jun 1, 2020

how are you testing it

By using speedtest websites which have Chinese server, we can see redirect in the result. Test with any one of the following:

I created this host to test the redirect rules, it uses the same rules as this PR

  • http://ec2-34-219-134-42.us-west-2.compute.amazonaws.com/
RewriteCond %{ENV:GEOIP_COUNTRY_CODE} ^CN$
RewriteCond %{HTTP_HOST} !cdn
RewriteRule ^(.*) https://mxnet.cdn.apache.org%{REQUEST_URI} [R,NC,L]

Please see this test result: https://www.webpagetest.org/result/200602_1B_4efffb1027dd235c69766571b04d658e/1/details/#waterfall_view_step1

@ys2843 ys2843 requested a review from szha June 2, 2020 00:34
@szha szha merged commit bd026a8 into apache:master Jun 2, 2020
@ys2843 ys2843 deleted the fix-geo-redirect-rules branch June 2, 2020 04:51
ys2843 added a commit to ys2843/incubator-mxnet that referenced this pull request Jun 2, 2020
yijunc pushed a commit to yijunc/incubator-mxnet that referenced this pull request Jun 9, 2020
AntiZpvoh pushed a commit to AntiZpvoh/incubator-mxnet that referenced this pull request Jul 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants