You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "johnslavik (via GitHub)" <gi...@apache.org> on 2025/12/03 23:12:21 UTC
[PR] Add check for break/continue/return in finally blocks [airflow]
johnslavik opened a new pull request, #59019:
URL: https://github.com/apache/airflow/pull/59019
https://docs.astral.sh/ruff/rules/jump-statement-in-finally/
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
This is a follow up to https://github.com/apache/airflow/pull/58488#discussion_r2585548176, where enabling SIM017 was initially proposed.
This PR, however, enables [B012](https://docs.astral.sh/ruff/rules/jump-statement-in-finally/), which is a more general rule that additionally catches `break` and `continue` in `finally` blocks -- which is an exhaustive list of what [PEP 765](https://peps.python.org/pep-0765/) prohibits.
CC @jscheffl
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add check for break/continue/return in finally blocks [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #59019:
URL: https://github.com/apache/airflow/pull/59019#issuecomment-3610404905
(I'd say similar like I am incrementally enabling all PLW rules in https://github.com/apache/airflow/pull/58116 with parallel PRs...)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add check for break/continue/return in finally blocks [airflow]
Posted by "johnslavik (via GitHub)" <gi...@apache.org>.
johnslavik commented on PR #59019:
URL: https://github.com/apache/airflow/pull/59019#issuecomment-3609294157
[3 fixes needed](https://github.com/apache/airflow/actions/runs/19911896616/job/57082826328?pr=59019) -- not bad!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add check for break/continue/return in finally blocks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59019:
URL: https://github.com/apache/airflow/pull/59019#issuecomment-3622381301
Cool find and waiting for reopening. That's an important rule to have.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add check for break/continue/return in finally blocks [airflow]
Posted by "johnslavik (via GitHub)" <gi...@apache.org>.
johnslavik commented on PR #59019:
URL: https://github.com/apache/airflow/pull/59019#issuecomment-3609300661
@jscheffl How would you like this resolved?
I'd fix B012 errors in separate PRs, then merge this one in. Just so that appropriate PRs are reviewed by proper owners.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add check for break/continue/return in finally blocks [airflow]
Posted by "johnslavik (via GitHub)" <gi...@apache.org>.
johnslavik closed pull request #59019: Add check for break/continue/return in finally blocks
URL: https://github.com/apache/airflow/pull/59019
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add check for break/continue/return in finally blocks [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #59019:
URL: https://github.com/apache/airflow/pull/59019#issuecomment-3610403539
Cool! Thanks for the initiative in enabling!
So in total I see (only) 3 errors, would have expected more. 1 of them is fixed with the PR from @AutomationDev85, the second is in a similar area. And the one in Fab should also be easily fixable in my view.
As there is no harm in B012 and actually I agree it is a bad practice I have nothing against it and would rather favor it to enable it as well. But it is valid, there might be other opinions. Therefore we have pull requests! If we wait that the other PR is opened and the other two are fixed (either in this PR or in separate in parallel, then rebase this one...) then we just leave it open for a few days for others to object.
It is not in a complexity that it would require a VOTE on devlist in my view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add check for break/continue/return in finally blocks [airflow]
Posted by "johnslavik (via GitHub)" <gi...@apache.org>.
johnslavik commented on PR #59019:
URL: https://github.com/apache/airflow/pull/59019#issuecomment-3612661068
I've had to delete the head repo -- will reopen this once the lints are fixed. Sorry for the hassle, had to move around stuff.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org