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

call FlowRunContext.model_rebuild() in hydrate_context #16628

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

Conversation

zzstoatzz
Copy link
Collaborator

@zzstoatzz zzstoatzz commented Jan 7, 2025

fixes EngineContext rebuild error, which when occurring was causing this downstream error, as the pydantic error could not be serialized

ray.exceptions.RaySystemError: System error: Failed to unpickle serialized exception
traceback: Traceback (most recent call last):
  File "/Users/nate/Library/Caches/uv/archive-v0/9aF8mmPcr6WgOvDDdAgDC/lib/python3.12/site-packages/ray/exceptions.py", line 51, in from_ray_exception
    return pickle.loads(ray_exception.serialized_exception)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: PydanticErrorMixin.__init__() missing 1 required keyword-only argument: 'code'

and assorted typing improvements

Copy link

codspeed-hq bot commented Jan 7, 2025

CodSpeed Performance Report

Merging #16628 will not alter performance

Comparing prefect-ray-fixes (d50e605) with main (fbffa5c)

Summary

✅ 3 untouched benchmarks

@zzstoatzz zzstoatzz changed the title ray call FlowRunContext.model_rebuild() in hydrate_context Jan 7, 2025
@zzstoatzz zzstoatzz added integrations Related to integrations with other services fix A fix for a bug in an existing feature labels Jan 7, 2025
@@ -81,6 +82,7 @@ def hydrated_context(
if flow_run_context := serialized_context.get("flow_run_context"):
flow = flow_run_context["flow"]
task_runner = stack.enter_context(flow.task_runner.duplicate())
FlowRunContext.model_rebuild(_types_namespace=_types)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

open to better ways to do this (if possible)

Copy link
Member

Choose a reason for hiding this comment

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

I think only Flow is needed for this rebuild, so it might be worthwhile only to import Flow and pass it into _types_namespace.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's what I expected as well, and did at first, but unfortunately that doesn't fix the problem. we see the same unpickleable error

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, well if we're importing _types, we might as well just import prefect.main which would allow us to remove the explicit model_rebuild call here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

and the circle continues

@zzstoatzz zzstoatzz marked this pull request as ready for review January 7, 2025 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for a bug in an existing feature integrations Related to integrations with other services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants