-
Notifications
You must be signed in to change notification settings - Fork 985
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
Project Links in left navbar display in reverse order from setup.py project_urls #3097
Comments
Thanks for the report! Especially the runscope capture, that's really nice. Since this is "working as expected" (i.e. the links are there), and since the spec doesn't say anything about ordering, I'm going to put the "Feature Request" label on this, which means that it's prioritized sometime after the completion of Milestone 6 (unless someone else wants to work on it in the meantime). |
Makes sense. The Runscope capture will likely expire in 30 days or so; if it's gone, it was pretty easy to make: twine upload --repository-url https://${YOUR_BUCKET}.runscope.net/pypi -u TEST -p TEST dist/* (with a free Runscope account, or substitute your preferred postbin service) |
I am pretty sure we just treat this as a dictionary so it might even change orders between page loads? Unless we added a sort in there somewhere. |
FWIW, there does seem to be an effort in models.Release.urls to ensure some ordering of urls is maintained—though it looks like this is more about Homepage at the top and Download at the bottom. And its test has a TODO to check the ordering. [Edit: sorry, models.Release.urls, not models.Project.urls] |
We're treating them as @medmunds Sounds like you're halfway towards solving this yourself. 🙂 The next step would be fixing that TODO and making the test fail (and also getting tests running locally), want to take a shot at it? Feel free to ask questions here if you have any. |
@di sure, I'll take a shot. (But probably won't get around to getting a dev environment set up before next week.) I don't see where Dependency or Release.project_urls specifies an order, and I don't see an obvious Dependency column to sort on anyway, so I'm guessing "newest to oldest" just happens to be what the DB is returning. (But I'm a SQLAlchemy novice. Am I missing something? Implicit primary key that could be used for order_by?) An easy, but brittle, fix is just reversing the project_urls returned from Release.urls. (Brittle as in DB-dependent, though no less deterministic than the current ordering. 🙂) A robust fix is probably adding a position column as part of "TODO: Move project URLs into their own table, since they are not actually a 'dependency'." (Which is probably more than I'm ready to bite off just yet.) |
No worries! Like I said, this isn't a huge priority.
Indeed, I'm pretty sure this is what's happening. Here is where they're created.
Yeah, which means that this probably needs to wait until post-legacy shutdown when we are more free to change the way things work in the DB. |
Make minimal change to display project URLs in same order they appear in package METADATA. Add specific test case. Also note more robust fix available with future schema change. Fixes pypi#3097.
Make minimal change to display project URLs in same order they appear in package METADATA. Add specific test case. Also note more robust fix available with future schema change. Fixes pypi#3097.
OK, opened a PR for the simple fix, and updated/added comments for the more-robust fix later when there are related changes going into the DB. (Btw, everything—code and tests—had already been using OrderedDicts, except for the one TODO line in the test case which deliberately suppressed the ordering because of this issue.) |
Please revisit fixing this issue, the PR looked good to me... |
(The earlier PR was a fragile workaround, and was withdrawn.) So the correct fix involves changing the DB schema to move Project URLs out of Release Dependencies, and adding a position column to support ordering. (See this TODO in models.py: "Move project URLs into their own table, since they are not actually a 'dependency'.") For the schema change: would it make sense to create a generalized ReleaseMetadata table (key/value/position) that could support other types of (non-dependency, possibly ordered) per-release metadata? Or just keep it simple with a ProjectURL table (url/position) for now. (I think Project-URL is the only metadata this would currently apply to.) @di? Incidentally, the tests for project URL ordering already exist, but right now they deliberately ignore order by casting OrderedDict to dict. (See this TODO in test_models.py.) |
I would just make it a dedicated table for project URLs. Alternatively you could do something like using a jsonb column or something, but you'd have to put your own validations in then to make sure the structure is sane. |
Any chance of fixing this? I discovered it myself and adapted, but especially now that dicts are ordered, it's baffling that it's reverse order. I noticed it again on https://pypi.org/project/asgiref which has urls of Changelog / Further Documentation / Documentation... :) |
We should probably just alphabetize this list for now instead, as there's nothing in the metadata specs, build tools, etc that supports explicitly ordered project URLs or guarantees ordering.
This might be true for the Python version you're using for your build, but it's not guaranteed to be true for all build environments for all artifacts on PyPI, and not the only thing that influences the order here. IMO, trying to continue to support this is fragile and likely just going to require more maintenance in the future. |
Looks to me that latest warehouse is sorting the URLs (except homepage) - I think this issue may therefore be resolved? |
They do seem sorted after Homepage now. It would be great to have control over the order, but the metadata formats don't specify them in an ordered way. |
Indeed, the order has been made explicit when this PR added a new table for them. |
The Project Links displayed in the left navbar appear to be in the opposite order as they're listed in setup
project_urls
(and resulting package METADATA).This isn't a huge issue, but some project urls are usually better starting points than others, so it would be nice to keep them ordered.
Example:
Result on test warehouse:
I've checked that twine is posting the project_urls fields in the same order as they're listed in setup.py, so I'm pretty sure this is occurring in Warehouse. Here's a full upload from twine captured in Runscope, if helpful (not real auth, of course).
twine version 1.9.1, pkginfo: 1.4.1, requests: 2.18.4, setuptools: 38.5.1, requests-toolbelt: 0.8.0, tqdm: 4.19.6, Python: 3.6.4
The text was updated successfully, but these errors were encountered: