add Geolimit failure redirect feature (short for NGB) #1067
Conversation
|
Will there be anyone to check my pull request? @dneuman64 |
|
|
||
| if (idx < 0) { | ||
| //this is a relative url, belongs to this ds | ||
| type = 1; |
There was a problem hiding this comment.
Setting type to 0 or 1 doesn't have an easy to understand meaning. Can you use names to describe the two modes? For example maybe "Relative URL" and "Absolute URL"?
There was a problem hiding this comment.
Thanks for comment. It makes sense, I will set it to string type.
|
yes, we will be looking into it soon. I am sorry we havent yet. As of right now it has conflicts, can you clean those up? |
|
ok, I will fix the new conflicts and update it soon! |
nbaoping
force-pushed
the
ngb-master
branch
3 times, most recently
from
473c53c to
b003cbf
Compare
|
I updated the code, will check it again? @dneuman64 @limited |
|
yes, will do today @nbaoping |
|
any way you could add a test or two that exercises the intended behavoir? |
| final Iterator<String> itr = dsMap.keySet().iterator(); | ||
| while (itr.hasNext()) { | ||
| final DeliveryService ds = dsMap.get(itr.next()); | ||
| String type = "INVALID_URL"; |
There was a problem hiding this comment.
Do you think it would be better to just not set this here and do ds.setGeoRedirectUrlType() for each case instead?
There was a problem hiding this comment.
yeah, I will add some tests for it
|
Some code was refactored, and more UTs were added, would you check it again? @dneuman64 @limited |
|
Thanks, I will take a look at it either today or tomorrow. One more ask: when you get a chance can you add some documentation explaining what it is and how to configure it? Something similiar to what you have in the PR description would be great. This page might be a good place for it: http://traffic-control-cdn.net/docs/latest/admin/traffic_router.html |
|
Hey @nbaoping this all looks good. I will merge once you add some docs. Thanks! |
|
Sorry for late, sure, I will add the doc |
|
Hi @dneuman64 , the doc was added, please have a look, thanks! |
|
|
||
| Overview | ||
| -------- | ||
| This feature is also called 'National GeoBlock' feature which is short for NGB feature. In this section, the 'NGB' will used for this feature. |
There was a problem hiding this comment.
"the 'NGB' will used for" -> "the acronym 'NGB' will be used for"
|
Thanks for reviewing that @limited! |
|
@limited thanks for your detail reviewing, I updated it, please have a look, thanks! |
add Geolimit failure redirect feature (short for NGB)
In the past, if the Geolimit check fails (for example, the client ip is not in the 'US' region but the geolimit is set to 'CZ + US'), the router will return 503 response; but with this feature, when the check fails, it will return 302 if the redirect url is set in the delivery service.
The Geolimit check failure has such scenarios:
And different configured redirect urls have different meanings:
http://<cache server name>.<delivery service's FQDN>/<configured relative path>'