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

feat(deps)!: switch to pydantic 2 #1777

Merged
merged 59 commits into from
Aug 9, 2024
Merged

Conversation

lengau
Copy link
Collaborator

@lengau lengau commented Jul 31, 2024

Includes the entirety of #1776

lengau added 30 commits July 30, 2024 21:06
BuildPlanner tests are still failing due to validation issues.
`build-for` now has to be a list, even with `all`.

So `build-for: all` in yaml becomes `build-for: [all]`
The only thing that remained using this module was its set of tests.
Also removes the related unused test fixtures.
These are leftover from before the craft-application move, but were used
for ensuring compatibility of the commands. Now that they're no longer
needed, we can eliminate them.
craft-application provides this so this one is redundant
Also removes the now-unused `format` module.
Everything from this function now exists in the AnalysisService.
Instead there's just a charmhub_config fixture that provides access to
the charmhub staging.
@lengau lengau force-pushed the work/1768/pydantic-2 branch from cb31611 to a359a25 Compare August 8, 2024 16:29
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

@lengau lengau force-pushed the work/1768/pydantic-2 branch from a359a25 to 1b42bc3 Compare August 8, 2024 16:33
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

assert err[0]["msg"] == "extra fields not permitted"


def test_partconfig_bad_type():
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer necessary because the source configuration validator failing will prevent the charm-requirements validator from running.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comes from #1776

@@ -46,14 +43,3 @@ def test_bundleplugin_get_build_commands(bundle_plugin, tmp_path):
f'mkdir -p "{str(tmp_path)}/parts/foo/install"',
f'cp -R -p -P {tmp_path}/parts/foo/build/* "{str(tmp_path)}/parts/foo/install"',
]


def test_bundleplugin_invalid_properties():
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer necessary as this is tested in craft-parts itself.

@@ -111,15 +110,6 @@ def test_get_build_commands(plugin, tmp_path):
]


def test_invalid_properties(plugin):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer necessary as this is done in craft-parts itself.

@lengau lengau marked this pull request as ready for review August 8, 2024 17:09
@lengau lengau requested review from mr-cal and tigarmo August 8, 2024 17:10
Copy link
Contributor

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@@ -64,8 +64,8 @@ def from_charm(cls, charm: Charm) -> Self:

Performs the necessary renaming and reorganisation.
"""
charm_dict = charm.dict(
include=const.METADATA_YAML_KEYS | {"title"},
charm_dict = charm.model_dump(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this have mode="json"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't need to here since we're converting Python types, but good question!

"""Preprocess any values that charmcraft infers, before attribute validation."""
if "type" not in values:
raise ValueError("Project type must be declared in charmcraft.yaml.")

return values

@pydantic.validator("parts", pre=True, always=True, allow_reuse=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always=True

so this needs to be handled on the field now right? didn't this change break anything?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation is still handled like the upstream one, but unfortunately this preprocessing can't work on the field itself :-/

Python 3.11 is the first version with `os.EX_OK`, but 3.12 is the
version used on noble.
@lengau lengau force-pushed the work/1768/pydantic-2 branch from 49f451d to 29f22f9 Compare August 8, 2024 19:58
On Windows, set the minimum to Python 3.11 as python 3.10 doesn't have
os.EX_OK
@lengau lengau force-pushed the work/1768/pydantic-2 branch from 1ba946a to 52cd8cb Compare August 9, 2024 22:35
@lengau lengau merged commit 70078b0 into feature/pydantic-2 Aug 9, 2024
12 checks passed
@lengau lengau deleted the work/1768/pydantic-2 branch August 9, 2024 22:38
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.

3 participants