You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@airflow.apache.org by "Gautam-Bharadwaj (via GitHub)" <gi...@apache.org> on 2026/03/17 06:15:35 UTC

[PR] Cleanup: Enforce keyword-only session in db_cleanup [airflow]

Gautam-Bharadwaj opened a new pull request, #63781:
URL: https://github.com/apache/airflow/pull/63781

Enforce keyword-only session parameters in db_cleanup utility functions and update tests to adhere to Airflow 3 standards.

---

##### Was generative AI tooling used to co-author this PR?

- [ ] Yes

---

* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.

--
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] Cleanup: Enforce keyword-only session in db_cleanup [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.

potiuk commented on PR #63781:
URL: https://github.com/apache/airflow/pull/63781#issuecomment-4074485740

   @Gautam-Bharadwaj This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   
   **Issues found:**
   - :x: **Pre-commit / static checks**: Failing: CI image checks / Static checks. Run `prek run --from-ref main` locally to find and fix issues. See [Pre-commit / static checks docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
   - :x: **mypy (type checking)**: Failing: CI image checks / MyPy checks (mypy-providers). Run `prek --stage manual mypy-providers --all-files` locally to reproduce. You need `breeze ci-image build --python 3.10` for Docker-based mypy. See [mypy (type checking) docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#mypy-checks).
   - :x: **Other failing CI checks**: Failing: check-newsfragment-pr-number. Run `prek run --from-ref main` locally to reproduce. See [static checks docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
   
   > **Note:** Your branch is **10 commits behind `main`**. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.
   
   **What to do next:**
   - The comment informs you what you need to do.
   - Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
   - There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
   - Maintainers will then proceed with a normal review.
   
   Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).


-- 
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] Cleanup: Enforce keyword-only session in db_cleanup [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.

potiuk commented on PR #63781:
URL: https://github.com/apache/airflow/pull/63781#issuecomment-4476903884

@Gautam-Bharadwaj — Removing the `ready for maintainer review` label and converting back to draft. **The branch now has merge conflicts with `main`** that surfaced after the label was added.

The label's contract is that the PR is ready for maintainer review — a regression like this means the PR temporarily isn't. Rebase your branch onto the latest `main`, resolve conflicts, then mark "Ready for review" again to re-enter the queue.

`git fetch upstream main && git rebase upstream/main`, resolve, `git push --force-with-lease`. See the [working-with-git docs](https://github.com/apache/airflow/blob/main/contributing-docs/10_working_with_git.rst).

No rush.

---

_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._

--
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] Cleanup: Enforce keyword-only session in db_cleanup [airflow]

Posted by "SameerMesiah97 (via GitHub)" <gi...@apache.org>.

SameerMesiah97 commented on code in PR #63781:
URL: https://github.com/apache/airflow/pull/63781#discussion_r2972071807


##########
airflow-core/tests/unit/utils/test_db_cleanup.py:
##########
@@ -741,6 +741,16 @@ def test_drop_archived_tables(self, mock_input, confirm_mock, inspect_mock, capl
         else:
             confirm_mock.assert_not_called()
 
+    def test_export_archived_records_positional_fails(self):

Review Comment:
   I don't have a strong opinion against including these tests but aren't they just testing python's built-in behavior when `*` is added to the method signature? 



##########
airflow-core/src/airflow/utils/db_cleanup.py:
##########
@@ -621,6 +621,7 @@ def run_cleanup(
 def export_archived_records(
     export_format: str,
     output_path: str,
+    *,
     table_names: list[str] | None = None,
     drop_archives: bool = False,
     needs_confirm: bool = True,

Review Comment:
   Why did you only make a subset of the parameters keyword-only for this method when you made all parameters for `drop_archived_tables` keyword-only?



-- 
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] Cleanup: Enforce keyword-only session in db_cleanup [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.

potiuk commented on PR #63781:
URL: https://github.com/apache/airflow/pull/63781#issuecomment-4593373052

   @Gautam-Bharadwaj This draft PR has been inactive for 76 days since the last triage comment and no response from the author. Closing to keep the queue clean.
   
   You are welcome to reopen this PR when you resume work, or to open a new one addressing the issues previously raised. There is no rush — take your time.
   
   ---
   
   _Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._


-- 
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] Cleanup: Enforce keyword-only session in db_cleanup [airflow]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.

Copilot commented on code in PR #63781:
URL: https://github.com/apache/airflow/pull/63781#discussion_r3025337004


##########
airflow-core/src/airflow/utils/db_cleanup.py:
##########
@@ -652,7 +653,10 @@ def export_archived_records(
 
 @provide_session
 def drop_archived_tables(
-    table_names: list[str] | None, needs_confirm: bool, session: Session = NEW_SESSION
+    *,
+    table_names: list[str] | None,
+    needs_confirm: bool,

Review Comment:
   `drop_archived_tables` is now fully keyword-only (`*, table_names, needs_confirm, session`), which is a larger breaking change than “keyword-only session”. If you only need `session` to be keyword-only (per Airflow core guideline), you can keep `table_names`/`needs_confirm` positional-or-keyword and introduce `*, session` instead.
   ```suggestion
       table_names: list[str] | None,
       needs_confirm: bool,
       *,
   ```



##########
airflow-core/tests/unit/utils/test_db_cleanup.py:
##########
@@ -741,6 +741,16 @@ def test_drop_archived_tables(self, mock_input, confirm_mock, inspect_mock, capl
         else:
             confirm_mock.assert_not_called()
 
+    def test_export_archived_records_positional_fails(self):
+        with pytest.raises(TypeError) as exc:
+            export_archived_records("csv", "path", ["table"], True, True, None)
+        assert "takes 2 positional arguments but 6" in str(exc.value)
+
+    def test_drop_archived_tables_positional_fails(self):
+        with pytest.raises(TypeError) as exc:
+            drop_archived_tables(["table"], True)
+        assert "takes 0 positional arguments but 2" in str(exc.value)

Review Comment:
   These tests assert on a specific CPython `TypeError` wording/argument count substring (e.g. "takes 2 positional arguments but 6"). The exact message format can vary across Python versions/implementations, making this brittle. Consider asserting only that a `TypeError` is raised, or use `pytest.raises(..., match=...)` with a looser regex that doesn’t depend on the exact count/message text.
   ```suggestion
           with pytest.raises(TypeError, match="positional arguments"):
               export_archived_records("csv", "path", ["table"], True, True, None)
   
       def test_drop_archived_tables_positional_fails(self):
           with pytest.raises(TypeError, match="positional arguments"):
               drop_archived_tables(["table"], True)
   ```



-- 
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] Cleanup: Enforce keyword-only session in db_cleanup [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.

potiuk closed pull request #63781: Cleanup: Enforce keyword-only session in db_cleanup
URL: https://github.com/apache/airflow/pull/63781


-- 
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