Unresolver: allow a full URL when un-resolving a domain #11632
Conversation
This also checks that the URL has a valid protocol.
stsewd
force-pushed
the
check-uri-protocol
branch
from
bbe0343 to
0638e30
Compare
There was a problem hiding this comment.
I left some questions about this PR.
I'm a little confused with the names here. We call this method unresolve_domain, but it could also receive a URL now. However, we have another method called unresolve_url. Shouldn't we standardize this and call unresolve_domain or unresolve_url depending on what the input we have?
With this changes, what is the difference between unresolve_url and unresolve_domain now?
unresolve_domain will resolve the domain only, everything else is ignored (it's in the docstring). |
readthedocs/core/unresolver.py
Outdated
| if not domain: | ||
| raise InvalidSubdomainError(domain) | ||
|
|
There was a problem hiding this comment.
Do we not want to put this check on line 562, since it's checking for the domain set there?
This also checks that the URL has a valid protocol. This is needed for readthedocs/readthedocs-corporate#1887.
This also checks that the URL has a valid protocol. This is needed for readthedocs/readthedocs-corporate#1887.
This also checks that the URL has a valid protocol. This is needed for https://github.com/readthedocs/readthedocs-corporate/pull/1887.