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

migrate prefect-docker to pydantic 2 #13697

Merged
merged 9 commits into from
May 31, 2024
Merged

migrate prefect-docker to pydantic 2 #13697

merged 9 commits into from
May 31, 2024

Conversation

aaazzam
Copy link
Contributor

@aaazzam aaazzam commented May 31, 2024

No description provided.

@aaazzam aaazzam marked this pull request as ready for review May 31, 2024 02:35
@aaazzam aaazzam requested review from desertaxle, zzstoatzz and a team as code owners May 31, 2024 02:35
@aaazzam aaazzam requested a review from zangell44 May 31, 2024 13:50
@@ -68,6 +62,9 @@ class ImagePullPolicy(enum.Enum):
NEVER = "Never"


VolumeStr = Annotated[str, AfterValidator(lambda v: ":" in v)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤌

setup.cfg Outdated
@@ -114,4 +114,4 @@ format = parsable
eval_annotations = yes
only_show_violations = yes
targets = 3.9-
exclusion_regex = ^src/prefect/_vendor/.*$|^src/prefect/utilities/compat\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^tests/workers/test_process_worker.py$|^tests/test_background_tasks.py$
exclusion_regex = ^src/prefect/_vendor/.*$|^src/prefect/utilities/compat\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^src/prefect/engine/runner\.py$|^src/prefect/engine/flow_runner\.py$|^src/prefect/engine/task_runner\.py$|^src/prefect/engine/executor\.py$|^src/prefect/engine/result\.py$|^tests/workers/test_process_worker.py$|^tests/test_background_tasks.py|^src/integrations/.*/tests/.*$
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why wouldn't we need the tests to pass vermin? They will run on each supported Python version, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vermin doesn’t exclude exceptiongroup as a package like it does typing_annotations, so it just reads ExceptionGroup and says “nuh uh that’s not valid on 3.9”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

though we should be a little less ham fisted with excluding all tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment on lines +220 to +227
with pytest.raises(ExceptionGroup) as exc:
async with DockerWorker(work_pool_name="test") as worker:
await worker.run(
flow_run=flow_run, configuration=default_docker_worker_job_configuration
)
assert len(exc.value.exceptions) == 1
assert isinstance(exc.value.exceptions[0], docker.errors.APIError)
assert "test error" in str(exc.value.exceptions[0])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting, what's the advantage of the ExceptionGroup here if we're just going to make sure it's a very precise exception anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we run tests in anyio threads and anyio > 4 now doesn’t throw errors it only throws exception groups

@aaazzam aaazzam merged commit b8dbceb into main May 31, 2024
7 of 12 checks passed
@aaazzam aaazzam deleted the prefect-docker-pydantic-v2 branch May 31, 2024 14:27
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.

None yet

2 participants