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 block reference load in async flow #15487

Merged

Conversation

GalLadislav
Copy link
Contributor

This PR fixes loading block references in async flows. For async flows it throws ParameterTypeError as the sync compatible Block.load_from_ref call does not runs the coroutine.

Related to no existing issue (or i could not find any) and i assumed that it might not need one.

Checklist

  • If this pull request adds new functionality, it includes unit tests that cover the changes

Copy link

codspeed-hq bot commented Sep 24, 2024

CodSpeed Performance Report

Merging #15487 will not alter performance

Comparing GalLadislav:fix-block-reference-load-in-async (e30ef8e) with main (323d82e)

Summary

✅ 3 untouched benchmarks

@desertaxle desertaxle added the fix A fix for a bug in an existing feature label Sep 25, 2024
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 for the fix @GalLadislav! I think we'll want to revisit this when working on #15008 to create sync and async versions of validate_parameters, but this makes sense to me for now.

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.

nice!

@zzstoatzz zzstoatzz merged commit 9d293ad into PrefectHQ:main Sep 25, 2024
32 checks passed
@GalLadislav GalLadislav deleted the fix-block-reference-load-in-async branch November 11, 2024 17:52
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.

4 participants