Split X-RTD-Version-Method header into two HTTP headers. #6907
Conversation
humitos
commented
Apr 14, 2020
humitos
commented
- Version-Method reflects how the version slug was obtained:
- external_domain: slug from external versions' domain (split after --)
- url: slug was taken directly from the URL
- Project-Method indicates how the project slug was obtained
- rtdheader: X-RTD-SLUG header
- subdomain: slug is the first part in the host before readthedocs.io
- cname: slug is from a custom domain
Split X-RTD-Version-Method header into two HTTP headers. * Version-Method reflects how the version slug was obtained: * external_domain: slug from external versions' domain (split after --) * url: slug was taken directly from the URL * Project-Method indicates how the project slug was obtained * rtdheader: X-RTD-SLUG header * subdomain: slug is the first part in the host before readthedocs.io * cname: slug is from a custom domain
|
This seems technically correct, but I don't know if it's useful. Having one header for all of these seems like the best UX, I don't want to be returning a million headers, so I'd just rename it |
readthedocs/proxito/views/mixins.py
Outdated
| if hasattr(request, 'cname'): | ||
| response['X-RTD-Version-Method'] = 'cname' | ||
| else: | ||
| response['X-RTD-Version-Method'] = 'url' |
There was a problem hiding this comment.
Let's remove this line entirely and only send it on external_domain, that removes a lot of my issue with it.
There was a problem hiding this comment.
If we don't send this header from Proxito, how would be the NGINX config? Do we need to add an if there to avoid checking for this variable?
There was a problem hiding this comment.
I'm not sure. I guess this is fine for now if we care, but I think for PR's, we know where the version came from, so I don't know if this is super useful. 🤷
