Skip to content

fix(console): don't warn on missing path dep in excluded group - #11018

Open
St4r4x wants to merge 2 commits into
python-poetry:mainfrom
St4r4x:fix/local-path-dep-group-aware-check
Open

fix(console): don't warn on missing path dep in excluded group#11018
St4r4x wants to merge 2 commits into
python-poetry:mainfrom
St4r4x:fix/local-path-dep-group-aware-check

Conversation

@St4r4x

@St4r4x St4r4x commented Aug 21, 2026

Copy link
Copy Markdown

Resolves: #10461

Problem

PathDependency.__init__ (in poetry-core) unconditionally logs a warning if a path/directory/file dependency's target doesn't exist, at the point the full pyproject.toml is parsed — before this CLI has determined which --with/--without/--only groups are actually active for the current install/sync invocation. So a path dependency in an excluded group (e.g. a dev-only local package that's never copied into a Docker image) gets flagged even though it was never going to be installed.

The already-correct, group-aware validation in Installer (installer.py:373, dep.validate(raise_error=not op.skipped)) still runs afterward and is unaffected — a path dependency that IS actually needed and IS missing still fails install/sync with a real error, exactly as before.

Fix

Added a suppressed_loggers mechanism to Command (parallel to the existing loggers list register_command_loggers already reads), letting a command raise specific loggers above WARNING for itself, unless run with --verbose/-vv/--debug. InstallCommand (and SyncCommand, which inherits from it) uses this to suppress poetry.core.packages.path_dependency's premature warning.

While testing, found and fixed a related issue: logger levels are global process state, so an earlier version of this raised the level but never reset it for a later command in the same process that doesn't ask for suppression. Application now tracks which logger names it's currently suppressing and resets any a new command doesn't ask for — relevant for anything running more than one command per process, this repo's own test suite included.

Nothing changed in poetry-core; this is scoped entirely to how this CLI configures logging per command.

Tests

  • tests/console/test_application.py: new test covering default/--verbose/-vv.

  • tests/console/commands/test_install.py: existing fixtures already cover the scenario. Ran the full suite deterministically and with randomized order (several times) specifically to rule out logger-state leakage across tests.

  • Added tests for changed code.

  • Documentation: none needed, internal logging fix with no user-facing API change.


Note on process: I worked on this with AI assistance (Claude) to investigate the root cause, write the fix, and write/run the tests. I reviewed and understand the changes and I'm the one deciding what's in this PR.

poetry-core's PathDependency.__init__ unconditionally warns if a
path/directory/file dependency's target doesn't exist, at pyproject.toml
parse time, before the CLI knows which --with/--without/--only groups
are active for this invocation. A path dependency in an excluded group
(e.g. a dev-only local package absent from a Docker image) was getting
flagged even though it was never going to be installed.

The already-correct, group-aware validation in Installer
(dep.validate(raise_error=not op.skipped)) still runs afterward and is
unaffected: a path dependency that IS needed and IS missing still
fails install/sync with a real error, exactly as before.

Add a suppressed_loggers mechanism to Command, letting a command raise
specific loggers above WARNING for itself unless run verbose/debug.
InstallCommand (and SyncCommand, which inherits from it) uses this to
suppress poetry.core.packages.path_dependency's premature warning.

Logger levels are global process state, so also track which logger
names are currently suppressed on Application and reset any a new
command doesn't ask for, so suppression from one command doesn't leak
into a later command in the same process (relevant for this repo's own
test suite, and for anyone using Application programmatically).

Resolves: python-poetry#10461

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • When resetting previously suppressed loggers in register_command_loggers, you currently set them to the global level, which can overwrite any prior per-logger configuration; consider capturing and restoring each logger’s original level (e.g., via a small mapping keyed by logger name) so that suppression is strictly temporary and does not alter existing logging configuration beyond the lifetime of the command.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- When resetting previously suppressed loggers in `register_command_loggers`, you currently set them to the global `level`, which can overwrite any prior per-logger configuration; consider capturing and restoring each logger’s original level (e.g., via a small mapping keyed by logger name) so that suppression is strictly temporary and does not alter existing logging configuration beyond the lifetime of the command.

## Individual Comments

### Comment 1
<location path="src/poetry/console/application.py" line_range="593-594" />
<code_context>
+        # Mutated in place (rather than `self._suppressed_logger_names = ...`)
+        # so the update lands on the shared ClassVar instead of shadowing it
+        # with a same-named instance attribute.
+        for name in Application._suppressed_logger_names - to_suppress:
+            logging.getLogger(name).setLevel(level)
+
+        for name in to_suppress:
</code_context>
<issue_to_address>
**issue (bug_risk):** Restoring suppressed loggers to `level` may unintentionally lower externally configured log levels.

This loop sets each previously suppressed logger to the command’s `level`, which can lower a logger that was externally configured to a stricter level (e.g., CRITICAL) down to WARNING/INFO on later, less-verbose commands. To avoid overriding stricter policies, consider either tracking which loggers were temporarily bumped to ERROR and only resetting those, or resetting with something like `max(existing_level, level)` instead of always applying `level`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +593 to +594
for name in Application._suppressed_logger_names - to_suppress:
logging.getLogger(name).setLevel(level)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Restoring suppressed loggers to level may unintentionally lower externally configured log levels.

This loop sets each previously suppressed logger to the command’s level, which can lower a logger that was externally configured to a stricter level (e.g., CRITICAL) down to WARNING/INFO on later, less-verbose commands. To avoid overriding stricter policies, consider either tracking which loggers were temporarily bumped to ERROR and only resetting those, or resetting with something like max(existing_level, level) instead of always applying level.

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.

Missing locale dev dependencies are reported although installend without dev

1 participant