-
Notifications
You must be signed in to change notification settings - Fork 233
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
Register import hook instead of directly wrapping when module is string #275
Comments
Do you mean: Thus:
Otherwise not really sure what you are asking for. |
Yeah it's possible to call |
I would have to think about it, it would be mixing what are currently separate bits of functionality which themselves are not dependent on each other. The current behaviour also possibly could not be changed to avoid impacts so perhaps would have to saying something like "this?", ie., use trailing "?" (convention sort of like optional chaining used in languages like Swift) to flag that should not do it immediately if not yet imported and instead setup lazy import hook instead. |
Thanks for the consideration @GrahamDumpleton! Agree that while I wouldn't expect it commonly, it is hard to be confident if making the change transparently. FWIW, syntax like a question mark to opt-in would make sense to me and add the convenience of going through the import hook. |
Currently, wrapping via e.g.
wrap_function_wrapper
import wrapped modules right awayhttps://github.com/GrahamDumpleton/wrapt/blob/develop/src/wrapt/patches.py#L17
I was wondering if it could make sense to instead register a import hook using
register_import_hook
. Conceptually,Before
After
I understand this would be a big difference in behavior but I wonder if there is any conceivable case where it could affect downstream code. Currently, calls to
wrap_
eagerly import the libraries, which means loading libraries to monkey patch which may not actually be imported by the application. Instead using an import hook would allow monkey patching only when a library is actually used which could have some memory benefits, etc. If a user passed a module in directly it would still behave as currently - a string for module seems like a good user intent to want "lazy behavior".It's not difficult to expect callers to use the import hook themselves so it's not a big deal, just wanted to bring up the idea in case it could make it easier for wrappers without causing regressions in behavior. Just for reference, this would make the behavior match closer e.g.
nodejs/import-in-themiddle
or Java classloader instrumentation in agents - I doubt that should be a priority for this library but just for context.The text was updated successfully, but these errors were encountered: