Skip to content
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

Introduce a new dependabot-related pip-compile GH actions workflow [pydantic saga] #765

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

eskultety
Copy link
Member

This should finally resolve the problem of pydantic async release cycles for pydantic and pydantic-core causing CI issues preventing merges of pydantic-related changes (see patch 2 for detailed explanation).
The effect of the workflow is illustrated here: eskultety#33 (comment)

Note, after the workflow, manual intervention is still going to be needed, because the only thing the workflow does is issue a comment, but then it's just a matter of applying the patch from the comment and force pushing to dependabot's branch and that should be it.

Depends on #764

Maintainers will complete the following section

  • Commit messages are descriptive enough
  • Code coverage from testing does not decrease and new code is covered
  • Docs updated (if applicable)
  • Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

Pydantic formats down URL types as "type casts" in the serialized
dictionary [1]. Apparently, that's by design and the docs advises to
use custom serializers for given fields [2].

This patch is necessary, because pydantic 2.10.2 changed their behavior
when it comes to handling URL types [3] and so once we bump the version
we'd hit a test failure requiring us to change the unit test to "cast"
some fields as AnyUrl rather than Url. Since these have no place in
plain dictionaries, this patch fixes the problem in the correct way by
the use of pydantic serializers.

[1] pydantic/pydantic#10998
[2] https://docs.pydantic.dev/latest/api/functional_serializers/#pydantic.functional_serializers.PlainSerializer
[3] https://pydantic.dev/articles/pydantic-v2-10-release#migrate-to-subclassing-instead-of-annotated-approach-for-pydantic-url-types

Signed-off-by: Erik Skultety <[email protected]>
This workflow is a direct consequence of the asynchronous release
schedule of pydantic and pydantic core and the fact that pydantic is
always pinned to a particular pydantic-core version. Dependabot doesn't
see these transitive relations and so can't properly update the
versions in this case (it always assumes the latest for every
dependency). This will naturally lead to broken CI making these version
updates impossible to merge.

Since our project directly only cares about pydantic and not
pydantic-core, we can ignore pydantic-core updates (future patch) and
run a dedicated workflow on every dependabot pull request that would
check whether changes to our requirements files are needed. If changes
are needed, the GitHub actions bot will comment on the pull request
that a change to these files are needed and provide a patch to the
reviewer to apply and update the pull request.

The workflow is only executed when changes to the requirements files
are proposed (realistically only by dependabot). Note that it's not
possible to specify the source branch as the workflow trigger, only the
target branch, and so that could not have been used as a better filter
for dependabot-proposed pull requests specifically. It is run using a
Python Alpine docker image, saves the git diff produced by pip-compile
to the default github actions environment followed by a github script
action that will pop the diff out of the environment and use it to
comment on the pull request.

References:
- https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#multiline-strings
- https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/using-conditions-to-control-job-execution
- https://github.com/actions/github-script?tab=readme-ov-file#comment-on-an-issue
- actions/github-script#247 (comment)
- actions/github-script#220 (comment)

Signed-off-by: Erik Skultety <[email protected]>
pydantic and pydantic-core have different release cycles and since a
pydantic release is always locked to a particular pydantic-core
version, a dependabot update to both will cause install conflicts
pretty much every time because dependabot doesn't see transitive
dependency relations.

This patch makes it so that pydantic-core's updates are ignored by
dependabot (because it's a transitive dependency for us) and we're only
going to consider pydantic. This however, will still break installs
because pip-compile needs to be invoked to re-pin this new pydantic
version to a correct pydantic-core version. That is handled by the
newly introduced github dependabot workflow 'dependabot-pipcompile.yml'
introduced in a past patch.

Signed-off-by: Erik Skultety <[email protected]>
@@ -88,7 +96,7 @@
:param download_url: The URL to download the artifact from.
"""

download_url: AnyUrl
download_url: Annotated[AnyUrl, PlainSerializer(lambda url: str(url), return_type=str)]

Check notice

Code scanning / CodeQL

Unnecessary lambda Note

This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
@@ -120,7 +128,7 @@
class LockfileArtifactMavenAttributes(BaseModel):
"""Attributes for a Maven artifact in the lockfile."""

repository_url: AnyUrl
repository_url: Annotated[AnyUrl, PlainSerializer(lambda url: str(url), return_type=str)]

Check notice

Code scanning / CodeQL

Unnecessary lambda Note

This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
@eskultety
Copy link
Member Author

Note that I also considered force pushing directly from the workflow to the same branch, but I originally faced some permissions issue. Admittedly, that could have been due to missing GH token permissions (addressed), but then I'd be most likely spawning a new concurrent gating job out of a different workflow just because we'd be force pushing a pip-compile change. Anyway, I reckoned such approach would have been obfuscating and opaque so I went with with a clearer approach requiring a bit of manual intervention though.

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.

1 participant