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

remove SimpleRepositoryPage #9166

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dimbleby
Copy link
Contributor

so far as I can see the only achievement of the SimpleRepositoryPage is to add an unwanted trailing slash, making it impossible to use files rather than directories as single page sources eg https://download.pytorch.org/whl/torch_stable.html

the addition of a trailing slash was made back in pre-history at 1900633. I can't tell what the intention was: but for what it's worth the two testcases in that commit survive today and still pass.

fixes #6885

@abn
Copy link
Member

abn commented Mar 16, 2024

For completeness, the trailing slash comes from PEP 503. The intention with the separation was to allow non compliant repos like this one. Seems somewhere along the lines that support went away causing the single page repos to fail after 1.2.

All URLs which respond with an HTML5 page MUST end with a / and the repository SHOULD redirect the URLs without a / to add a / to the end.

The html page was indeed originally intended to abstract away both cases.

@abn
Copy link
Member

abn commented Mar 16, 2024

See #5517

@dimbleby
Copy link
Contributor Author

https://storage.googleapis.com/jax-releases/jax_releases.html has absolute links to wheels, so the bogus trailing slash made no difference in the example that #5517 tested against.

single-page repositories with relative links have never worked.

@abn
Copy link
Member

abn commented Mar 17, 2024

#5517 makes use of SinglePageRepository not SimpleRepositoryPage. Guess relative page handing for SinglePageRepository is actually what is broken f the torch repos do not actually work.

Fwiw, I am not saying SimpleRepositoryPage as it stands is worth keeping around, but merely stating that it was separated out from HTMLPage as a PEP 503 simple repo is different to a "single page" repo.

@abn
Copy link
Member

abn commented Mar 17, 2024

the addition of a trailing slash was made back in pre-history at 1900633. I can't tell what the intention was: but for what it's worth the two testcases in that commit survive today and still pass.

For this one, I am fairly certain this was added to avoid redirects, which I think we handle much better these days implicitly.

@dimbleby
Copy link
Contributor Author

Guess relative page handing for SinglePageRepository is actually what is broken f the torch repos do not actually work

the SinglePageRepository gets a SimpleRepositoryPage:

def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage:

It really is the handling of the SimpleRepositoryPage that breaks eg the torch repo

@dimbleby dimbleby force-pushed the remove-simple-repository-page branch 2 times, most recently from f51ff21 to 11d09e7 Compare March 17, 2024 10:22
@dimbleby
Copy link
Contributor Author

I added a testcase to show that this fixes something (and protect against regressions)

@dimbleby dimbleby force-pushed the remove-simple-repository-page branch 4 times, most recently from f0f31b5 to 8e2bf84 Compare March 18, 2024 00:23
@dimbleby dimbleby force-pushed the remove-simple-repository-page branch from 8e2bf84 to 2df6ba7 Compare March 25, 2024 20:18
@neersighted neersighted force-pushed the remove-simple-repository-page branch 2 times, most recently from 1c8ab4a to 0836e48 Compare March 25, 2024 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support files, not just directories, as single page sources
2 participants