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 recursive cache for private modules #2928

Draft
wants to merge 4 commits into
base: danielsola/se-251-make-auto-cache-plugin
Choose a base branch
from

Conversation

dansola
Copy link
Contributor

@dansola dansola commented Nov 13, 2024

Why are the changes needed?

See #2912 for more context.

Make caching easier to use in flytekit by reducing cognitive burden of specifying cache versions.

What changes were proposed in this pull request?

In this change we add a class to recursively explore the any functions and class methods defined in the users repo and called within a function of interest. The hash of each user defined function or method computed and hashed together to generate a single unique hash.

This covers the following scenarios:

  • Nesting of functions in functions in functions, and so on.
  • Imports that happen at the global level or within a task/function/method defnition.
  • Imports where the module is imported and the function or class is referenced. e.g. import my_module, print(my_module.var)
  • Imports where function or class itself is imported. e.g from my_module import var
  • If user defined class is used, we recursively hash all the methods.
  • If an external library is used (as determined by site-packages), we stop the recursive search and don't hash that external functions contents

How was this patch tested?

Since get_version is already tested in #2912, we just ensure that the correct callables are extracted for hashing. We set up a mock package and make sure all of the above conditions are satisfied.

Will eventually separate tests out so failures can be more easily diagnosed.

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

@dansola
Copy link
Contributor Author

dansola commented Nov 13, 2024

cc @eapolinario @cosmicBboy @granthamtaylor

This is the second caching method added so far in the series of draft PRs for auto caching.

Some work that probably still needs to be done here is logging around when some dependency is not resolved or a configuration to let all dependencies to be logged for transparency.

Signed-off-by: Daniel Sola <[email protected]>
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (danielsola/se-251-make-auto-cache-plugin@50552a9). Learn more about missing BASE report.

Additional details and impacted files
@@                             Coverage Diff                             @@
##             danielsola/se-251-make-auto-cache-plugin    #2928   +/-   ##
===========================================================================
  Coverage                                            ?   76.06%           
===========================================================================
  Files                                               ?      200           
  Lines                                               ?    20784           
  Branches                                            ?     2667           
===========================================================================
  Hits                                                ?    15809           
  Misses                                              ?     4260           
  Partials                                            ?      715           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

1 participant