Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve redirects feature #4221

Closed
agjohnson opened this issue Jun 11, 2018 · 14 comments · Fixed by #10881
Closed

Improve redirects feature #4221

agjohnson opened this issue Jun 11, 2018 · 14 comments · Fixed by #10881
Assignees
Labels
Feature New feature Needed: design decision A core team decision is required

Comments

@agjohnson
Copy link
Contributor

agjohnson commented Jun 11, 2018

We brought up the idea of using our YAML file for redirects. There are a number of problems with this though, as well as many existing problems.

Current problems:

  • If the application is down, redirects don't work Update: still technically true, but the application has been super solid
  • If we wanted to make a Azure storage hosted version, we don't have an application or ngnix to fall back on for redirects Update: Proxito is the layer in front of S3 storage now, and applies hosting 🪄 up front
  • Redirects are project level currently -- this is still a large issue

New problems:

  • Keeping per-version redirects in YAML makes a lot of sense, but per-project redirects are still valid and do not make sense in YAML (We discussed this in depth at Config file: support for multiple config files in a project #8811)
  • Because of ^ we need 2 sets of redirects -- the UX would probably be to make current redirects "Project Redirects"
  • Making per-version redirect model is okay, as we can update the database per-version on build, but the point above about azure redirects is valid still. No it's not!

A more current description of where this work is:

  • We need to finalize a design decision on how we're technically exposing project level redirects in a configuration file
  • There are a few extensions folks have authored to integrate with our API, so this could also be a path forward, though it does feel like a native feature is long overdue here
@agjohnson agjohnson added Needed: design decision A core team decision is required Priority: low Low priority labels Jun 11, 2018
@agjohnson agjohnson added this to the YAML File Completion milestone Jun 11, 2018
@agjohnson agjohnson added the Feature New feature label Jun 11, 2018
@humitos
Copy link
Member

humitos commented Mar 12, 2020

Some things have changed since this issue was created. I think it's a good moment to revive this conversation. The most important decision to make, IMHO, is the scope of the redirects: project vs. version.

I'd vote to have everything in the config file and de-prioritize any web UI for this work. I do see these options we could implement redirects on YAML file:

  • only respects redirects from the default version YAML file and ignore any new redirect created in other versions (including PR) --similar to what we have now, but changing the web UI by the YAML in the default version
  • give priority to redirects from the current version requested, and fallback to project-level redirects

@ericholscher
Copy link
Member

Yea, I think we can follow a similar approach here, where we can have project-level redirects defined, and we use the default_version's project-level redirects. That at least gives us a specific way to make it work, and lets us keep it in YAML at the version-level which would be nice.

@stsewd stsewd changed the title Redirects in YAML Redirects in configuration file Jun 1, 2020
@pradyunsg
Copy link

pradyunsg commented Mar 3, 2021

Ahoy! This would be great for pip, and anything to have these redirects allowed at the "root" level would be ideal.

We'd like to start using this for things like error messages and survey links, to make them easier to reference and to have them look more official. This is especially useful for things like https://pip.pypa.io/surveys/backtracking, which are really nice. :)

@ericholscher
Copy link
Member

@pradyunsg I don't think you need it in the config file for this? You could set it up on the RTD admin. I'm 👍 on doing more around redirects tho.

@pradyunsg
Copy link

pradyunsg commented Mar 3, 2021

We've got those configured in the admin pages (that's why that link works 😉).

It'd be nice to have this be something that we can have under version control and accept PRs for (like a config file)! That'll help make it easier for contributors to help add to this, and all the advantages that come with that. :)

@pradyunsg
Copy link

pradyunsg commented Mar 6, 2021

I just noticed that I didn't make a note of why I brought this up again: I'm doing a rewrite of pip's documentation basically, and this would help make that easier to review for folks + to roll out those changes.

Additionally (as I mentioned) we're starting to use this for "official links" in tweets / error messages etc. That means that the project-owners become the blockers for contributors' PRs that need things like this which... isn't ideal? (One of our contributors even filed a PR for trying to add a page with a client-side redirect, because none of us had come around to doing this on the server side: pypa/pip#9680).

@pradyunsg
Copy link

Okay, so... as it turns out, I'm very interested in this. What can I do to help move this forward?

@ericholscher
Copy link
Member

@pradyunsg The big questions are around how to make the UX make sense. What are project-level vs. version-level redirects, how do we handle them both, etc. I think in the past we've had project-level stuff uses the default version, but I don't love that since it's pretty confusing and non-obvious to users. If we end up with 2 different Redirect UI's, it gets really wonky as well.

@agjohnson
Copy link
Contributor Author

Just a note: while we're figuring out how to best make this a native feature, there are a few options for authors here:

@stsewd stsewd moved this from Planned to In progress in 📍Roadmap May 13, 2022
@stsewd stsewd moved this from In progress to Planned in 📍Roadmap May 19, 2022
@ericholscher ericholscher changed the title Redirects in configuration file Improve redirects feature Jun 7, 2022
@ericholscher
Copy link
Member

We had a lot more discussion about this here: #9188 -- I think we can probably close this as a dupe, but not 100% sure they are solving the same things.

@agjohnson
Copy link
Contributor Author

Yeah one of these should be closed probably. I believe we landed on not implementing this in the config file, at least to start.

@pradyunsg
Copy link

Thanks for the confirmation on that @agjohnson! I've ended up implementing this via a script for pip in that case; that uses the RTD API. :)

https://github.com/pypa/pip/blob/main/.readthedocs-custom-redirects.yml
https://github.com/pypa/pip/blob/main/tools/update-rtd-redirects.py

@stsewd
Copy link
Member

stsewd commented Sep 28, 2023

Some related issues:

Eric also mentioned having a way to have more specific redirects per domain, I like how CF does this (*.rtd.com/, */foo). Haven't thought much about how we will be able to support these, but at least matching things per-domain could work.

@ericholscher
Copy link
Member

ericholscher commented Oct 3, 2023

Yea, I think #9614 is part of what we hit when migrating EA domains. It would be nice to be able to apply a redirect to a domain without it effecting all domains (like PR builds), so that you can redirect an old domain cleanly. For EA, we wanted a ethicalads.io/* to www.ethicalads.io/$1 redirect, but couldn't get it working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature Needed: design decision A core team decision is required
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants
@ericholscher @humitos @agjohnson @pradyunsg @stsewd and others