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

Add PrefectFuture implementation for flow runs #17234

Merged
merged 6 commits into from
Feb 21, 2025
Merged

Conversation

desertaxle
Copy link
Member

@desertaxle desertaxle commented Feb 21, 2025

This PR adds a PrefectFuture implementation for tracking flow run execution that works similarly to the PrefectDistributedFuture for task runs. To support the operation of the new PrefectFlowRunFuture, this PR also adds a FlowRunWaiter to monitor the execution of flow runs via events.

To make the inheritance hierarchy of PrefectFuture make a little more sense, I also added a PrefectTaskRunFuture class to hold some of the implementations that were on PrefectFuture so that PrefectFuture can be purely an interface and not a Frankenstein class. The implementations on PrefectFutrue have been deprecated and should be removed in a later release. This should only affect users that might be subclassing PrefectFuture.

This new future type will be used to track flow runs submitted to remote infrastructure via a worker.

Related to #17194

Copy link

codspeed-hq bot commented Feb 21, 2025

CodSpeed Performance Report

Merging #17234 will not alter performance

Comparing flow-run-watcher (68fcc8d) with main (a7e94ad)

Summary

✅ 2 untouched benchmarks

@desertaxle desertaxle marked this pull request as ready for review February 21, 2025 17:58
return self._task_run_id

@property
def state(self) -> State:
"""The current state of the task run associated with this future"""
warnings.warn(
"The state property of PrefectFuture is deprecated and will be removed in a future release. "
"If you are subclassing PrefectFuture, please implement the state property in your subclass.",
Copy link
Member

Choose a reason for hiding this comment

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

is it worth pointing these folks to the new class?

Copy link
Member

Choose a reason for hiding this comment

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

and is the plan for these to raise NotImplemented errors in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I can point to the new class. The plan is to remove these methods from PrefectFuture and let subclasses manage their implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated in 68fcc8d

Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

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

lgtm!

@desertaxle desertaxle merged commit fdc2b8a into main Feb 21, 2025
48 checks passed
@desertaxle desertaxle deleted the flow-run-watcher branch February 21, 2025 21:16
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