Skip to content

Conversation

@clockback
Copy link
Contributor

@clockback clockback commented Jul 24, 2025

Summary

Changing BLE001 (blind-except) so that it does not flag except clauses which include logging.critical(..., exc_info=True).

Test Plan

It passes the following (whereas the main branch does not):

$ cargo run -p ruff -- check somefile.py --no-cache --select=BLE001
# somefile.py

import logging


try:
    print("Hello world!")
except Exception:
    logging.critical("Did not run.", exc_info=True)

Related: #19519

clockback added a commit to clockback/pelican that referenced this pull request Jul 24, 2025
Note that two inline suppressions are used. I expect, provided that my
[PR](astral-sh/ruff#19520) is merged in, that
these suppressions will become superfluous in a later version of Ruff.
@ntBre ntBre self-requested a review July 24, 2025 13:06
@ntBre ntBre linked an issue Jul 24, 2025 that may be closed by this pull request
@ntBre ntBre added the rule Implementing or modifying a lint rule label Jul 24, 2025
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks! This makes sense to me. @MichaReiser does this need to go out in preview? blind-except (BLE001) is a stable rule.

I just had one minor docs nit, and could we add a test case or two? Maybe one example with exc_info without a lint and one without exc_info with a lint. These could go at the end of this file and basically be copies of the error case here:

try:
pass
except Exception:
logging.error("...", exc_info=True)

@github-actions
Copy link
Contributor

github-actions bot commented Jul 24, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser
Copy link
Member

MichaReiser commented Jul 24, 2025

Thanks! This makes sense to me. @MichaReiser does this need to go out in preview? blind-except (BLE001) is a stable rule.

Our versioning policy is a bit vague here:

  • The scope of a stable rule is significantly increased
  • The intent of the rule changes

Given that this reduce violations, I'd say it's probably okay but it might be good to do a quick grep for # noqa: BLE001 to see if you can find any suppressions specifically for this pattern. If there are many -> preview, otherwise go ahead

@ntBre
Copy link
Contributor

ntBre commented Jul 24, 2025

I went through two pages of search results on GitHub and didn't find any instances of this pattern. Along with the clean ecosystem report, I think this is good to go once we add tests.

@clockback clockback requested a review from ntBre July 25, 2025 08:58
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thank you!

@ntBre ntBre changed the title Change BLE001 to permit logging.critical(..., exc_info=True). [flake8-blind-except] Change BLE001 to permit logging.critical(..., exc_info=True). Jul 25, 2025
@ntBre ntBre merged commit 72fdb7d into astral-sh:main Jul 25, 2025
35 checks passed
@clockback clockback deleted the logging-critical-bypass-blind-except branch July 25, 2025 22:08
clockback added a commit to clockback/pelican that referenced this pull request Jul 31, 2025
Note that two inline suppressions are used. I expect, provided that my
[PR](astral-sh/ruff#19520) is merged in, that
these suppressions will become superfluous in a later version of Ruff.
clockback added a commit to clockback/pelican that referenced this pull request Jul 31, 2025
Note that two inline suppressions are used. I expect, provided that my
[PR](astral-sh/ruff#19520) is merged in, that
these suppressions will become superfluous in a later version of Ruff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow logging.critical to bypass BLE001

3 participants