Skip to content

Conversation

@JacobCoffee
Copy link
Member

@JacobCoffee JacobCoffee commented Oct 8, 2025

Description

  • Fixes issue with /downloads/windows when trying to parse a non-standard relase object version

@zooba
Copy link
Member

zooba commented Oct 8, 2025

The root cause that got us here was that I ticked "show on download page" for a PyManager release, which I shouldn't have done. https://github.com/python/pythondotorg/blob/main/downloads/managers.py#L16 would have filtered it out and we never would've hit this path.

@zooba
Copy link
Member

zooba commented Oct 8, 2025

Not disagreeing with making the function not crash, to be clear. Maybe it's just as good to make get_version return an empty string in the "not a real version" case?

@JacobCoffee
Copy link
Member Author

whichever you prefer, or if it's just better to document/know that show on download page shouldn't be enabled for pymanager releases? you all are the release experts :D

@JacobCoffee JacobCoffee marked this pull request as ready for review October 8, 2025 14:59
@JacobCoffee JacobCoffee requested a review from ewdurbin as a code owner October 8, 2025 14:59
@zooba
Copy link
Member

zooba commented Oct 8, 2025

if it's just better to document/know ...

It never is! 😆

Let's at least make sure that checking the box doesn't bring down the site, and that unchecking it (which I have already done) fixes up any weird display on the page. Then we can rely on documentation from there, I think.

@JacobCoffee
Copy link
Member Author

@zooba does f809fd1 work for you? or did i misunderstand

➜ docker compose run --rm web ./manage.py test downloads
[+] Creating 2/2
 ✔ Container pythondotorg-redis-1     Running              0.0s 
 ✔ Container pythondotorg-postgres-1  Running              0.0s 

System check identified 2 issues (0 silenced).
.................................................................
----------------------------------------------------------------------
Ran 65 tests in 10.299s

OK
Destroying test database for alias 'default'...

@zooba
Copy link
Member

zooba commented Oct 8, 2025

Yeah, that's the other approach I was thinking of. Both are fine, but I think the latter is a bit cleaner/safer over time.

@JacobCoffee JacobCoffee merged commit e8ac397 into main Oct 8, 2025
6 checks passed
@JacobCoffee JacobCoffee deleted the downloads-invalid-version-pslit-fix branch October 8, 2025 19:03
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.

4 participants