Add PyPI source type and migrate pip-installed Python plugins#2441
Add PyPI source type and migrate pip-installed Python plugins#2441stefanvanburen merged 7 commits intomainfrom
Conversation
Fixes #2440 Add a pypi source type to the fetcher so plugins distributed as Python packages can track releases from PyPI rather than GitHub tags. The fetcher queries https://pypi.org/pypi/{name}/json, skips yanked and pre-release versions, and respects the existing ignore_versions and max_version filters. Migrate four plugins whose Dockerfiles install via pip to the new source type: - connectrpc/python (protoc-gen-connect-python) - community/nipunn1313-mypy (mypy-protobuf) - community/nipunn1313-mypy-grpc (mypy-protobuf) - community/danielgtaylor-betterproto (betterproto) Leave protocolbuffers/python, protocolbuffers/pyi, and grpc/python on github, as all three compile C++ from source using Bazel and version against the upstream monorepo tag rather than any PyPI package. Add table-driven tests for fetchPyPI covering yanked releases, empty file lists, Python-style pre-release strings (e.g. 2.0.0b7), and the ignore_versions/max_version filters. Mock data is shaped after the real mypy-protobuf PyPI API response.
pkwarren
left a comment
There was a problem hiding this comment.
Overall this looks good - just a couple things.
| goProxyURL = "https://proxy.golang.org" | ||
| npmRegistryURL = "https://registry.npmjs.org" | ||
| mavenURL = "https://repo1.maven.org/maven2" | ||
| // docs: https://warehouse.pypa.io/api-reference/json.html |
There was a problem hiding this comment.
This link isn't working for me - should it be updated to https://docs.pypi.org/api/json/?
| request, err := http.NewRequestWithContext( | ||
| ctx, | ||
| http.MethodGet, | ||
| fmt.Sprintf("%s/%s/json", c.pypiBaseURL, strings.TrimPrefix(name, "/")), |
There was a problem hiding this comment.
Do we expect PyPI.Name to ever start with /?
| var data struct { | ||
| Releases map[string][]struct { | ||
| Yanked bool `json:"yanked"` | ||
| } `json:"releases"` |
There was a problem hiding this comment.
The releases key on a project's json output is marked as deprecated: https://docs.pypi.org/api/json/#get-a-project
Should we switch to a different API to get this information?
| } | ||
| yanked := true | ||
| for _, file := range files { | ||
| if !file.Yanked { |
There was a problem hiding this comment.
It sounds like the yanked value can be a string or a bool:
yanked: An optional key which may be either a boolean to indicate if the file has been yanked, or a non empty, but otherwise arbitrary, string to indicate that a file has been yanked with a specific reason. If the yanked key is present and is a truthy value, then it SHOULD be interpreted as indicating that the file pointed to by the url field has been “Yanked” as per PEP 592.
Address review feedback from #2441: - Update docs URL to https://docs.pypi.org/api/simple/ - Use the Simple API (/simple/{name}/ with Accept: application/vnd.pypi.simple.v1+json) instead of the legacy JSON API, whose releases key is deprecated - Handle yanked as json.RawMessage: per PEP 691 the field is absent/false when not yanked, or a non-empty string reason when yanked - Add pypiVersionFromFilename to extract version from wheel/sdist filenames (handles both hyphenated and normalized underscore forms) - Add pypiFileYanked helper for the string-or-false yanked check - Update tests to use Simple API response shape and add TestPyPIVersionFromFilename covering both wheel and sdist formats
This reverts commit fb4290b.
Address review feedback from #2441: - Use the Simple Repository API (/simple/{name}/ with Accept: application/vnd.pypi.simple.v1+json) instead of the legacy JSON API, whose releases key is marked deprecated - Update docs URL to the Simple Repository API spec - Handle yanked per the spec: absent/false = available; true or any non-empty string reason = yanked (PEP 592) - Add pypiVersionFromFilename to extract version from wheel/sdist filenames, handling both hyphenated and normalized underscore forms - Add pypiFileYanked helper for the bool-or-string yanked check - Update tests to use Simple API response shape, add TestPyPIVersionFromFilename covering both wheel and sdist formats
This requires us to parse filenames for versions (it's not supported in the API); yanking happens so rarely that it doesn't seem like something we need to support. (Our previous approach of following GitHub releases would have this same issue.)
Fixes #2440.
Add a pypi source type to the fetcher so plugins distributed as Python packages can track releases from PyPI rather than GitHub tags. The fetcher queries
https://pypi.org/pypi/{name}/json, skipsyanked(edit: yanked out the yanked handling) and pre-release versions, and respects the existing ignore_versions and max_version filters.Migrate four plugins whose Dockerfiles install via pip to the new source type:
connectrpc/python(protoc-gen-connect-python)community/nipunn1313-mypy(mypy-protobuf)community/nipunn1313-mypy-grpc(mypy-protobuf)community/danielgtaylor-betterproto(betterproto)Leave
protocolbuffers/python,protocolbuffers/pyi, andgrpc/pythonon github, as all three compile C++ from source using Bazel and version against the upstream monorepo tag rather than any PyPI package.