Remove unnecessary sync async compat code from flowspy #14766
Conversation
cicdw
added
the
development
CodSpeed Performance ReportMerging #14766 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
I think load_flow_from_entrypoint has traditionally been called from a thread in async functions because it reads from disk when loading the flow. That disk access is likely fast enough that it's not worth the overhead and complexity of involving a thread in the cases where load_flow_from_entrypoint is used, so this LGTM!
| @@ -1887,10 +1883,8 @@ async def load_flow_from_flow_run( | |||
| run_logger.debug( | |||
| f"Importing flow code from module path {deployment.entrypoint}" | |||
| ) | |||
| flow = await run_sync_in_worker_thread( | |||
There was a problem hiding this comment.
I just remembered that I added some logic to run_sync_in_worker_thread to handle @sync_compatible functions at the module level (e.g. Block.load). We will likely need a new solution for that with this run_sync_in_worker_thread call removed.
There was a problem hiding this comment.
I see, good test!! I'll poke at this and see if I can simplify the current code and if not, I'll just close this PR
There was a problem hiding this comment.
Going to close for now, the refactors I've tried feel weird.
I noticed in working on a bug that this code was unnecessarily wrapped in sync/async compatibility stuff so this PR removes it.
🧹 🧹 🧹