flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard

try removing multiple imports to resolve fast-mode issue

Open novahow opened this issue 1 year ago • 2 comments

Tracking issue

https://github.com/flyteorg/flyte/issues/4716

Why are the changes needed?

Please refer to may example in https://github.com/flyteorg/flyte/issues/4716#issuecomment-1893423240

When we have a python file that contains workflows, tasks, and launch plans and try to run that in fast mode, we get an error: AssertionError: The cached values aren't the same as the current call arguments

This happens because when executing the file, the launch_plan's get_or_create method is invoked by the following snippet multiple times, thus the LaunchPlan.CACHE stores a workflow. After that when we're really creating a plan with standard_scale_launch_plan = LaunchPlan.get_or_create(, the workflow of the plan is different from the workflow of the cached plan, which I haven't figured out the reason.

What changes were proposed in this pull request?

My workaround is removing exec_module in the try block and import_module in the exception block so that get_or_create won't be invoked multiple times

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

  • [ ] I updated the documentation accordingly.
  • [ ] All new and existing tests passed.
  • [ ] All commits are signed-off.

Related PRs

Docs link

novahow avatar Jan 17 '24 07:01 novahow

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

welcome[bot] avatar Jan 17 '24 07:01 welcome[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (892b474) 85.77% compared to head (f144c96) 85.81%. Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2110      +/-   ##
==========================================
+ Coverage   85.77%   85.81%   +0.03%     
==========================================
  Files         313      313              
  Lines       23500    23503       +3     
  Branches     3512     3511       -1     
==========================================
+ Hits        20158    20169      +11     
+ Misses       2734     2726       -8     
  Partials      608      608              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 17 '24 07:01 codecov[bot]