Proxito: pass unresolved domain in request

[stsewd]

Make use of the unresolved domain from the request instead of each of the attributes we are setting in the request from the middleware, we aren't using any of those on .com, so we are safe removing them here.

This is on top of #9974.

[stsewd]

if we don't care about keeping the old names, we could also just use unresolved_domain.origin.name

[humitos]

I think nobody should be using this. We've used internally to know where we were getting this value from. I'm fine using new names here, but probably worth double checking with @ericholscher in case he has other concerns about this.

[ericholscher]

Yea, this is mostly just for debugging on our side. I don't think anything should be depending on it, but validatehttp would be the only place I'd think.

[stsewd]

Only the canonicalize attributes are set here now, but I think we should be able to move these to the serve_docs view, since there is the only place we make use of them, maybe the http to https redirect could be done at the middleware level.

[humitos]

Shouldn't we have a bigger object? I'd like to inject one and only one object to the request with a known structure. As an example:

class DataRequest:
  unresolved_domain
  canonicalize
  # ... others we will add in the future

What do you think?

[stsewd]

We are using these attributes in the docs view only, moving all the logic to the docs view serving should be easier

readthedocs.org/readthedocs/proxito/views/serve.py

Lines 141 to 160 in f607db6

	canonicalize_redirect_type = getattr(request, "canonicalize", None)
	if canonicalize_redirect_type:
	try:
	# A canonical redirect can be cached, if we don't have information
	# about the version, since the final URL will check for authz.
	if not version and self._is_cache_enabled(final_project):
	self.cache_request = True
	return self.canonical_redirect(
	request=request,
	final_project=final_project,
	version_slug=version_slug,
	filename=filename,
	redirect_type=canonicalize_redirect_type,
	is_external_version=is_external,
	)
	except InfiniteRedirectException:
	# Don't redirect in this case, since it would break things
	pass

Once that is moved, not sure if there is anything else that we will need to inject in the request, but if we have the need, I'd be fine creating another object.

[humitos]

Looks good! I left some suggestions to simplify it a little more.

[humitos]

I think nobody should be using this. We've used internally to know where we were getting this value from. I'm fine using new names here, but probably worth double checking with @ericholscher in case he has other concerns about this.

[humitos]

Shouldn't we have a bigger object? I'd like to inject one and only one object to the request with a known structure. As an example:

class DataRequest:
  unresolved_domain
  canonicalize
  # ... others we will add in the future

What do you think?

[humitos]

🚀