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

Lazy import #63073

Closed
wants to merge 6 commits into from
Closed

Lazy import #63073

wants to merge 6 commits into from

Conversation

rhansen
Copy link
Contributor

@rhansen rhansen commented Nov 17, 2022

What does this PR do?

  • Defines a new lazy_import function that acts like Python's import statement but defers module load until a module attribute is read or deleted. This function is unrelated to and independent of (not a wrapper around) the salt.loader.* stuff.
  • Converts some of the existing deferred import statements (import statements inside a function that are evaluated when the function is called) to lazy imports.
  • Converts some eager imports to lazy imports to avoid circular imports with planned features (YAML loading and dumping improvements #62932)

The new lazy_import function is a copy of https://docs.python.org/3/library/importlib.html#implementing-lazy-imports except:

  • This version checks if the module has already been imported.
  • This version supports lazy loading of parent modules (the example given in the Python documentation always eagerly loads any parent modules).
  • When importing parent.child, the child attribute of the parent module is set to the parent.child module object (like the import statement does).

What issues does this PR fix or reference?

See #62932 (comment) for context.

Behavior Changes

No behavior changes are expected. The new salt.utils.lazy.lazy_import function is meant to be a drop-in replacement of the Python import statement. In other words, this:

import salt.utils.lazy
_baz = salt.utils.lazy.lazy_import("foo.bar.baz")

behaves the same as:

import foo.bar.baz as _baz

except:

  • The actual read of foo/__init__.py, foo/bar/__init__.py, and foo/bar/baz.py—and the execution of their contents—does not happen until an attribute of _baz is read or deleted (assuming the modules are not already loaded).
  • Relative imports are not (yet) supported.

Merge requirements satisfied?

Commits signed with GPG?

No

@rhansen rhansen marked this pull request as ready for review November 17, 2022 04:57
@rhansen rhansen requested a review from a team as a code owner November 17, 2022 04:57
@rhansen rhansen requested review from twangboy and removed request for a team November 17, 2022 04:57
@rhansen rhansen force-pushed the lazy-import branch 4 times, most recently from ec4ba9e to 8d7b246 Compare November 22, 2022 05:41
twangboy
twangboy previously approved these changes Nov 28, 2022
@rhansen
Copy link
Contributor Author

rhansen commented Nov 29, 2022

(last push was a no-change rebase onto current master)

salt/utils/lazy.py Outdated Show resolved Hide resolved
@rhansen
Copy link
Contributor Author

rhansen commented Dec 1, 2022

@whytewolf said in #62932 (comment):

what is concerning is the magic importing wrapping esp considering you are wrapping the loader which is already a magic import system in another magic import system. your lazy loading at a bare min needs to go through a SEP. see https://github.com/saltstack/salt-enhancement-proposals

I'll start a SEP for this. However, I want to note that the above characterization of this PR is not accurate. The new salt.utils.lazy.lazy_import function is unrelated to and independent of (not a wrapper around) the salt.loader.* stuff. It is meant to be a drop-in replacement of the Python import statement. In other words, this:

import salt.utils.lazy
_baz = salt.utils.lazy.lazy_import("foo.bar.baz")

behaves the same as:

import foo.bar.baz as _baz

except:

  • The actual read of foo/__init__.py, foo/bar/__init__.py, and foo/bar/baz.py—and the execution of their contents—does not happen until an attribute of _baz is read or deleted (assuming the modules are not already loaded).
  • Relative imports are not (yet) supported.

The new lazy_import function is a copy of https://docs.python.org/3/library/importlib.html#implementing-lazy-imports except:

  • This version checks if the module has already been imported.
  • This version supports lazy loading of parent modules (the example given in the Python documentation always eagerly loads any parent modules).

The `salt.loader` module is intentially imported late in
`salt.config.call_id_function()` to avoid a circular import error.
However, `salt.loader` is also used in `salt.config.mminion_config()`,
so import it there as well.  This avoids an error if
`mminion_config()` is called before `call_id_function()`, or if the
import statement in `call_id_function()` is removed in the future.
The delayed `import` statements match `import` statements at the top
of the file so they are effectively no-ops.
For rationale, see the documentation for the new function.

Future commits will use this to replace existing delayed `import`
statements, and to defer some more module loads to avoid circular
imports.
This makes it possible to import these affected modules from core code
without introducing a circular import.
@rhansen
Copy link
Contributor Author

rhansen commented Dec 19, 2022

Closing in favor of the more limited PR #63335.

@rhansen rhansen closed this Dec 19, 2022
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.

2 participants