-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Improve stubsabot logic for finding the first release with a py.typed
file
#9600
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
Conversation
scripts/stubsabot.py
Outdated
normalised_versions_to_version_strings = { | ||
packaging.version.Version(version_string): version_string for version_string in pypi_info.releases | ||
} | ||
# We should be able to safely count on this being a non-empty iterable, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why so much new code is needed, and the comment gives me pause. What if instead we put if release.version.is_prerelease: continue
in the while loop above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That won't work because in the while
loop we're iterating over the releases in reverse order, so if we continue
, that means that we've already gone past the first non-prerelease with a py.typed
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can simplify by iterating over the releases forwards instead of backwards, but that will entail making many more network requests than is strictly necessary. But it would be simpler, and maybe we don't care about spamming PyPI with loads of network requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually I think you're right and I'm not talking any sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably do need some additional changes because this function seems to assume there is at least one py.typed release, which will no longer be the case. We may have to ignore prereleases somewhere else too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there are any changes needed elsewhere:
-
SQLAlchemy
has had lots of prereleases that includedpy.typed
files, but stubsabot only tried to mark our stubs as obsolete when they released a non-prerelease that had apy.typed
file. It just got the body and title of the PR incorrect. -
Everything seems to work fine when I run the script locally.
-
This function is only called if the "latest" release according to PyPI has a
py.typed
file in it:
Lines 420 to 425 in 0024dc4
is_obsolete = await release_contains_py_typed(latest_release, session=session) if is_obsolete: first_release_with_py_typed = await find_first_release_with_py_typed(pypi_info, session=session) relevant_version = version_obsolete_since = first_release_with_py_typed.version else: relevant_version = latest_version I think, if PyPI is working correctly, it won't claim that a prerelease is the "latest" release, which is why we didn't get stubsabot trying to mark our stubs for SQLAlchemy as obsolete until a non-prelease with a
py.typed
file is released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I missed the fact that SQLAlchemy 2.0 has actually been released. That makes sense now.
…`py.typed` file" This reverts commit b1a9468.
Addresses some of the confusion seen in #9595