-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Support files, not just directories, as single page sources #6885
Comments
I think we could have better logic here, but wouldn't dropping |
@neersighted Unfortunately no b/c the server returns
|
Interesting... Looks like a S3-compatible object store. I suppose we didn't consider that use case (servers that don't serve an index page) when designing this feature -- the title should probably be more along the lines of "support files, not just directories, as single page sources." It might be slightly more involved than not adding a trailing slash -- I'd invite you to explore and write some more robust tests for this is you are interested. |
I see. I'm happy to help as much as I can. What kind of tests are you looking for? It seems more involved now since we're not dealing with servers that don't serve index pages. In the meantime, any workaround? |
As far as tests go, ideally some fixtures/HTTP mocking that reproduce the path structure you're having problems with. After introducing those failing tests, you can more confidently iterate on fixing this. It could be as simple as the trailing slash, or it could be more complex -- adding as many test cases (for reasonable/real world structures/deployments) as you can think of and fixing the ones that fail is the best way to figure out if a more holistic fix is needed, or if it's just the trailing slash. Regarding workarounds, a reverse proxy rewriting URLs seems like the easiest/fastest to deploy; if we do have a code fix for this in Poetry, I personally would not be opposed to backporting a fix to the 1.2 branch so we can get it out sooner. |
@neersighted Let me know where I should be adding these test cases, and I’ll see what I can do! |
Has anything happened on this front since late-OCT? I'm having exactly same issue with adding mmcv-full. Adding it directly via |
That's #2415. Unfortunately, code does not appear from nothing; what you see on GitHub is representative of the progress this feature request has made. |
Sorry, but I fail to see how these 2 are related. I managed to fix this particular issue with 2 lines of code - many thanks to @yukw777 for pinpointing where the URL is created, so it only took me 10 minutes of setting up local poetry version rather than couple of hours (probably) trying to find where the URLs are born. Anywho, another, somewhat, related issue we've been experiencing was when |
#2415 describes what you are complaining of -- only |
So, do I understand it correctly, Also, if I offer a simple fix to THIS particular sub-issue ( |
You have the meaning completely inverted:
|
Alright, I'll make a PR then. I haven't done any testing besides running our massive repo through this modified version of poetry with switch from |
@Maciej-Zwolinski Sorry I dropped the ball on this one, but I'm glad my trace has been helpful to you. Please let me know if you need any help with your PR! |
just not adding the trailing slash seems like the correct fix, the danger is that there are users out there who are unknowingly relying on the trailing slash and will be broken by it not being there. IMO best rip the bandage: make the right fix, say something about it in the changelog, done. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
-vvv
option) and have included the output below.Issue
I'm trying to install
mmcv-full
. They provide wheels via a single page link source, which I have added as a source in mypyproject.toml
. Unfortunately, this page has relative paths with double dots, and poetry fails to resolve the correct wheel URLs. I believe the issue lies here:poetry/src/poetry/repositories/link_sources/html.py
Lines 60 to 61 in 337e90f
Once a trailing slash is added, (https://download.openmmlab.com/mmcv/dist/cu113/torch1.12/index.html => https://download.openmmlab.com/mmcv/dist/cu113/torch1.12/index.html/),
urllib.parse.urljoin()
fails to properly join the urls here:poetry/src/poetry/repositories/link_sources/html.py
Line 37 in 337e90f
Basically, we shouldn't always add trailing slashes to URLs.
The text was updated successfully, but these errors were encountered: