-
Notifications
You must be signed in to change notification settings - Fork 358
feat(pip): Add 'pythonInspectorVerbose' option to enable detailed resolver output #11058
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
base: main
Are you sure you want to change the base?
feat(pip): Add 'pythonInspectorVerbose' option to enable detailed resolver output #11058
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11058 +/- ##
============================================
- Coverage 57.42% 57.41% -0.01%
Complexity 1703 1703
============================================
Files 346 346
Lines 12835 12837 +2
Branches 1215 1216 +1
============================================
Hits 7370 7370
- Misses 4998 5000 +2
Partials 467 467
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
So, does it log passwords in pain text? |
Yes, currently it does. |
Then I think think should be make very clear in the comment for the introduced property. And the commit message should also refer to that and ideally contain the link to your newly created issue. |
e547640 to
e8ca63a
Compare
|
|
||
| /** | ||
| * If "true", enables verbose logging in `python-inspector`. | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the KDoc warn about the plaintext passwords being logged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #11058 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a section in the commit message to be cautions when enabling this option, especially until python-inspector does do no masking of the password.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add it to this code comment. It's too hidden in the commit message for such an important note.
| * If "true", enables verbose logging in `python-inspector`. | ||
| */ | ||
| @OrtPluginOption(defaultValue = "false") | ||
| val verbose: Boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we give this a Python inspector specific name?
(So, that we do not mix this up with Ort code verbosity)
maybe: pythonInspectorVerbose ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, makes sense, added it.
e8ca63a to
bb614ba
Compare
Introduce a 'pythonInspectorVerbose' flag in the PIP plugin configuration to enable more informative output from python-inspector. When set to true, it shows additional details such as dependency resolution steps, environment data, repository configuration, and used credentials. This helps diagnose repository access and configuration issues. Please note that passwords are currently displayed in plain text in the output of python-inspector. Therefore, please activate this new switch with caution. A feature request to mask passwords in the output has already been submitted: aboutcode-org/python-inspector#254 Signed-off-by: klw1imb <[email protected]>
bb614ba to
195f52c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I explicitly disabled this flag in #10624.
Until this issue is fixed in the Python Inspector, I don't think that we should give ORT users the possibility to use the --verbose in any way.
I'm also not convinced that warning about this in a code comment or in the commit message is enough to handle this issue.
So, how to proceed with this @MarcelBochtler, @wkl3nk? Should we close this PR for now? |
Introduce a 'pythonInspectorVerbose' flag in the PIP plugin configuration to enable more informative output from python-inspector. When set to true, it shows additional details such as dependency resolution steps, environment data, repository configuration, and used credentials. This helps diagnose repository access and configuration issues.
Please note that passwords are currently displayed in plain text in the output of python-inspector. Therefore, please activate this new switch with caution. A feature request to mask passwords in the output has already been submitted:
aboutcode-org/python-inspector#254