Proxito: refactor canonical redirects #10069
Conversation
stsewd
force-pushed
the
refactor-canonical-redirects
branch
from
e368e05 to
00ec8fb
Compare
stsewd
force-pushed
the
refactor-canonical-redirects
branch
from
00ec8fb to
1ab1759
Compare
stsewd
requested review from
ericholscher and
humitos
and removed request for
benjaoming
Nice! |
| use_custom_domain = self._use_cname(canonical_project) | ||
| custom_domain = canonical_project.get_canonical_custom_domain() |
There was a problem hiding this comment.
What about overriding .get_canonical_custom_domain() instead? Would that be doable and simpler?
benjaoming
changed the title
Logic around redirects won't change, or at least in a way that breaks the current expected behavior. Canonical redirects always were first, user redirects were second. |
|
This should be ready for another round of review, added a glossary of the terms we use for each URL part at https://github.com/readthedocs/readthedocs.org/pull/10069/files#diff-92614ae090281fddaae958813072070fe6fe35b6491babadaab557c05898b8ab. |
| @@ -648,6 +649,17 @@ def proxied_static_path(self): | |||
| """Path for static files hosted on the user's doc domain.""" | |||
| return f"{self.proxied_api_host}/static/" | |||
|
|
|||
| @property | |||
| def custom_path_prefix(self): | |||
There was a problem hiding this comment.
I feel these name changes a lot easier to read and follow 👍🏼
| @@ -5,6 +5,41 @@ Module in charge of serving documentation pages. | |||
|
|
|||
| Read the Docs core team members can view the `Proxito design doc <https://github.com/readthedocs/el-proxito/blob/master/docs/design/architecture.rst>`_ | |||
|
|
|||
| URL parts | |||
There was a problem hiding this comment.
Agreed, really happy to see this.
readthedocs/proxito/README.rst
Outdated
| URL | ||
| |------------------------------------------------| | ||
| path | ||
| |---------------------| | ||
| https://docs.readthedocs.io/en/latest/index.html | ||
| |-------|-------------------|--|------|----------| | ||
| protocol | | | | | ||
| domain | | | | ||
| language | | | ||
| version | | ||
| filename |
There was a problem hiding this comment.
This is great. Though this is varying a bit from urlparse in some ways which make sense, but we should be explicit if we don't care about the other parts (eg. query & fragment).
urlparse("scheme://netloc/path;parameters?query#fragment")
https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlparse
readthedocs/proxito/README.rst
Outdated
| URL | ||
| |------------------------------------------------| | ||
| path | ||
| |---------------------| | ||
| https://docs.readthedocs.io/en/latest/index.html | ||
| |-------|-------------------|--|------|----------| | ||
| protocol | | | | | ||
| domain | | | | ||
| language | | | ||
| version | | ||
| filename |
There was a problem hiding this comment.
This is great. Though this is varying a bit from urlparse in some ways which make sense, but we should be explicit if we don't care about the other parts (eg. query & fragment).
urlparse("scheme://netloc/path;parameters?query#fragment")
https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlparse
Implementing the plan from #10065.
Closes #10065