Taragolis commented on code in PR #33846:
URL: https://github.com/apache/airflow/pull/33846#discussion_r1314714882


##########
airflow/jobs/backfill_job_runner.py:
##########
@@ -656,8 +656,6 @@ def _per_task_process(key, ti: TaskInstance, session):
                             _per_task_process(key, ti, session)
                             try:
                                 session.commit()
-                                # break the retry loop
-                                break

Review Comment:
   We step on the field: `tabs` vs `spaces` and `"` vs `'`.
   
   According to the [documentation for 
python](https://docs.python.org/3/tutorial/errors.html#handling-exceptions)
   
   ```
   The try … except statement has an optional else clause, which, when present, 
must follow all except clauses. 
   It is useful for code that must be executed if the try clause does not raise 
an exception.
   ```
   
   Personally I do not know the tool which automatically replace it, so I can't 
see how we can't prevent new code without `else` clause appear in codebase. 
UnboundLocalError should in theory prevented by ruff or mypy anyway



##########
airflow/jobs/backfill_job_runner.py:
##########
@@ -656,8 +656,6 @@ def _per_task_process(key, ti: TaskInstance, session):
                             _per_task_process(key, ti, session)
                             try:
                                 session.commit()
-                                # break the retry loop
-                                break

Review Comment:
   We step on the field: `tabs` vs `spaces` and `"` vs `'`.
   
   According to the [documentation for 
python](https://docs.python.org/3/tutorial/errors.html#handling-exceptions)
   
   ```
   The try … except statement has an optional else clause, which, when present, 
must follow all except clauses. 
   It is useful for code that must be executed if the try clause does not raise 
an exception.
   ```
   
   Personally I do not know the tool which automatically replace it, so I can't 
see how we can't prevent new code without `else` clause appear in codebase. 
UnboundLocalError should in theory prevented by ruff or mypy anyway



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to