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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
