Skip to content
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

Log stderr warnings as warnings #772

Merged
merged 7 commits into from
Feb 20, 2025
Merged

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Feb 12, 2025

Description

Closes #770

@maresb maresb requested a review from a team as a code owner February 12, 2025 10:25
Copy link

netlify bot commented Feb 12, 2025

Deploy Preview for conda-lock ready!

Name Link
🔨 Latest commit 49a1f7a
🔍 Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/67ae1d3fa0bf6700081ebb33
😎 Deploy Preview https://deploy-preview-772--conda-lock.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@maresb maresb force-pushed the fix-stderr-logging branch from aecc14f to bb6b911 Compare February 12, 2025 10:26
@maresb
Copy link
Contributor Author

maresb commented Feb 12, 2025

@ctcjab, could you please review this?

@maresb
Copy link
Contributor Author

maresb commented Feb 12, 2025

The last commit is chasing a flake unrelated to #770.

@ctcjab
Copy link

ctcjab commented Feb 12, 2025

LGTM, thanks for the quick fix! 🙌

log_level = logging.WARNING
elif log_level == logging.WARNING:
if not line.startswith(" "):
log_level = logging.ERROR
Copy link

Choose a reason for hiding this comment

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

Just to make sure, will it always be true that any non-indented line that the subprocess writes to stderr (that does not succeed a line beginning with "warning") is actually an error?

If that's a safe bet, please ignore, but if there's still a nontrivial chance of false positives, is it worth exposing some option to control this, as an escape hatch?

Copy link
Contributor Author

@maresb maresb Feb 12, 2025

Choose a reason for hiding this comment

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

Thanks for the feedback. I fleshed it out quite a bit more, making the log level detection more robust, and also giving you two levels of escape hatches: CONDA_LOCK_SUBPROCESS_STDERR_DEFAULT_LOG_LEVEL and CONDA_LOCK_SUBPROCESS_STDERR_LOG_LEVEL_OVERRIDE.

@maresb
Copy link
Contributor Author

maresb commented Feb 12, 2025

Hmm, now conda create --dry-run --json is randomly emitting {}\n before the actual JSON output. 🙁 (Seemingly unrelated to the current changes.)

Copy link

@ctcjab ctcjab left a comment

Choose a reason for hiding this comment

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

Thanks, the new escape hatches are a great improvement!

@maresb maresb force-pushed the fix-stderr-logging branch from 9ebaead to 49a1f7a Compare February 13, 2025 16:26
@maresb
Copy link
Contributor Author

maresb commented Feb 20, 2025

The bad JSON output seems to have been due to the now-yanked mamba v2.0.6.

@maresb maresb merged commit 2da5924 into conda:main Feb 20, 2025
56 checks passed
@maresb maresb deleted the fix-stderr-logging branch February 20, 2025 13:12
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.

conda-lock emits spurious errors from libmamba warnings
2 participants