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

Fix load_flow_argument_from_entrypoint to work with async flows #13716

Merged
merged 7 commits into from
Jun 4, 2024

Conversation

elisalimli
Copy link
Contributor

@elisalimli elisalimli commented May 31, 2024

Fixes #13765

The load_flow_argument_from_entrypoint function in Prefect doesn't support finding async functions (AsyncFunctionDef) decorated with @flow.

What We Expect:
It should be able to find both async functions and regular functions (FunctionDef) decorated with @flow.

@elisalimli elisalimli requested a review from a team as a code owner May 31, 2024 15:35
@serinamarie
Copy link
Contributor

serinamarie commented May 31, 2024

Hi @elisalimli thanks for opening your first PR to prefect! Giving the workflows a run now, and I think @desertaxle would probably be the best one to take a final look at this :)

Update: You probably want to install pre-commit hooks. Please see our contributing guide.

@desertaxle
Copy link
Member

Thanks for the PR @elisalimli! Could you please add a test case to cover this case?

@elisalimli
Copy link
Contributor Author

Thanks for the quick response @desertaxle and @serinamarie.

@desertaxle I have added a test case in 60b5602

Copy link
Contributor

@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!

cc @desertaxle

@elisalimli
Copy link
Contributor Author

This issue blocks us as a team right now, therefore we would be glad if you could merge within today/tomorrow. Thanks in advance!

cc @zzstoatzz @desertaxle @serinamarie

@elisalimli elisalimli requested a review from zzstoatzz June 3, 2024 11:58
@aaazzam
Copy link
Contributor

aaazzam commented Jun 4, 2024

Likely related to #13765

Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

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

Thanks!

@desertaxle desertaxle changed the title chore: Update AST traversal to include async function definitions in … Fix load_flow_argument_from_entrypoint to work with async flows Jun 4, 2024
@desertaxle desertaxle added the fix A fix for a bug in an existing feature label Jun 4, 2024
@desertaxle desertaxle merged commit 9bcabe3 into PrefectHQ:main Jun 4, 2024
26 of 27 checks passed
desertaxle pushed a commit that referenced this pull request Jun 4, 2024
…3716)

Co-authored-by: Serina Grill <42048900+serinamarie@users.noreply.github.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Async flow deployment regression in 2.19.3
5 participants