Skip to content

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

Merged
merged 3 commits into from
Jan 28, 2023

Conversation

AlexWaygood
Copy link
Member

Addresses some of the confusion seen in #9595

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,
Copy link
Member

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?

Copy link
Member Author

@AlexWaygood AlexWaygood Jan 28, 2023

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.

Copy link
Member Author

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

Copy link
Member Author

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

Copy link
Member

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.

Copy link
Member Author

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 included py.typed files, but stubsabot only tried to mark our stubs as obsolete when they released a non-prerelease that had a py.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:

    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.

Copy link
Member

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.

@AlexWaygood AlexWaygood marked this pull request as draft January 28, 2023 17:27
@AlexWaygood AlexWaygood marked this pull request as ready for review January 28, 2023 17:48
@AlexWaygood AlexWaygood merged commit 9cd20ce into main Jan 28, 2023
@AlexWaygood AlexWaygood deleted the stubsabot-obsoletion branch January 28, 2023 18:03
Avasam pushed a commit to Avasam/typeshed that referenced this pull request Feb 1, 2023
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.

2 participants