-
Notifications
You must be signed in to change notification settings - Fork 397
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
import commands only when dependencies are available #606
import commands only when dependencies are available #606
Conversation
@micheleAlberto Would you mind updating your commit to include a Since this is multiple commits, the easiest way to do that would be using a |
This lays the groundwork for shipping a version of Memray where these dependencies are optional. Signed-off-by: Michele Alberto <[email protected]>
fee9e81
to
c6882c4
Compare
Signed-off-by: Michele Alberto <[email protected]>
Signed-off-by: Michele Alberto <[email protected]>
Signed-off-by: Michele Alberto <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #606 +/- ##
==========================================
+ Coverage 92.55% 92.96% +0.41%
==========================================
Files 91 95 +4
Lines 11304 11423 +119
Branches 1581 2092 +511
==========================================
+ Hits 10462 10619 +157
+ Misses 837 804 -33
+ Partials 5 0 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thank you for the work you put in here, @micheleAlberto, but after some reflection, I've decided against going ahead with this approach. The risk that this approach introduces is that, because the set of reporters to be exposed in any given environment is determined implicitly by whether they can be successfully imported, we might introduce a new dependency on a library that's not guaranteed to be installed, and accidentally trigger the removal of a reporter that we meant to include in our default set. This case isn't as far fetched as it might seem, and some of these dependencies are quite subtle. For instance, Going further, another problem with making detection of optional libraries implicit rather than explicit is that we don't have a good way to notify the user of the missing dependency. I think what we want is for I do appreciate the work that you put in here, and it's certainly helped me solidify my understanding of the problem and what a solution needs to look like, but I don't think it's a good starting point to get to the best solution. Sorry! |
Issue number of the reported bug or feature request: #557
Describe your changes
This PR allows the
memray
cli to run with a reduced set of dependencies. The cli will only list commands that are supported by installed packages and ignore the commands that require additional packages.Testing performed
without textual
results in
without jinja2
results in
without jinja2 and textual
results in
Additional context
Add any other context about your contribution here.