Ensure user code never sees our 3rd party imports - #994
Open
godlygeek wants to merge 5 commits into
Open
Conversation
This is also used by the `summary` reporter, which is a CLI report and not a TUI report. Moving this breaks an import time dependency from `memray summary` on `textual`. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Whenever the `rich` module may or may not be needed by one of our modules, delay the import until the point where we know whether we will need it. Continue importing it unconditionally whenever every happy, non-error path that imports a module will always need `rich`. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Import `LiveCommand` inside the functions that actually start the live interface, so that the import of `textual` and `rich` is not visible to the user's application, only to the sibling process running the TUI. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Rather than having the `memray` CLI eagerly import every subcommand before even attempting to parse the command line, switch it to understand that the first non-option argument is a subcommand name, and to only import the module that the specific subcommand being invoked comes from. Fall back to importing all subcommands when no command is given, or when we need one parser that understands all commands (like for the manpage generation). Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Add tests ensuring that the three third-party Python libraries we currently depend on (jinja2, rich, textual) are not imported in the same process as the user's code, neither by `memray run` nor by `memray attach`. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Contributor
Author
|
@lkollar This lays the groundwork for what I was describing in #893 (comment) (and is something I've wanted to clean up for a long time, anyway...) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #994 +/- ##
==========================================
- Coverage 92.45% 92.39% -0.07%
==========================================
Files 101 101
Lines 13211 13223 +12
Branches 477 477
==========================================
+ Hits 12214 12217 +3
- Misses 997 1006 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Import
jinja2,rich, andtextuallazily so that they aren't loaded intosys.modulesbefore running user code inmemray runormemray attach.