Refactor deprecated SQLA files in cli folder #59630
Conversation
boring-cyborg
bot
added
area:CLI
area:dev-tools
backport-to-v3-1-test
airflow-core/tests/unit/cli/commands/test_connection_command.py
Outdated
Show resolved
Hide resolved
|
Thanks @Prab-27 for review. |
| cli_action_loggers.default_action_log(**metrics) | ||
|
|
||
| log = session.scalar(select(Log).order_by(Log.dttm.desc())) | ||
| log = session.execute(select(Log).order_by(Log.dttm.desc())).scalars().first() |
There was a problem hiding this comment.
I think this (and the one below) should keep the scalar pattern. That matches a bit better to what we want to do here, and is significantly easier to read IMO.
There was a problem hiding this comment.
Yes, all session.execute().scalars() calls can be modified to session.scalars()
There was a problem hiding this comment.
Thank for the advice. I'll modify the code according to the comments above.
|
|
||
| # Verify team was NOT deleted (invalid input treated as No) | ||
| team = self.session.query(Team).filter(Team.name == "confirm-invalid").first() | ||
| team = self.session.execute(select(Team).where(Team.name == "confirm-invalid")).scalars().first() |
There was a problem hiding this comment.
Same here too (there are a few more above too)
| pool_command.pool_set(self.parser.parse_args(["pools", "set", "foo", "1", "test"])) | ||
| pool_command.pool_delete(self.parser.parse_args(["pools", "delete", "foo"])) | ||
| assert self.session.query(Pool).count() == 1 | ||
| assert self.session.execute(select(func.count()).select_from(Pool)).scalar_one() == 1 |
There was a problem hiding this comment.
for func.count we can also use session.scalar(select(func.count()).select_from(Pool)) == 1
WDYT ?
| ^airflow-core/tests/unit/cli/commands/test_team_command\.py$| | ||
| ^airflow-core/tests/unit/cli/commands/test_connection_command\.py$| | ||
| ^airflow-core/tests/unit/cli/commands/test_pool_command\.py$| | ||
| ^airflow-core/tests/unit/utils/test_cli_util\.py$| |
There was a problem hiding this comment.
1 - Could you please check these added files ?
2 - since we are only adding files we don't need to use \ here just ^filename.py$| is sufficient (ex - line no. 428)
4 participants
related: #59402
unit/utils/test_cli_util.py #59464
cli/commands/test_pool_command #59463
cli/commands/test_connection_command #59462
cli/commands/test_team_command #59460
cli/commands/test_dag_command.py #59454
cli/commands/test_task_command #59451
cli/commands/test_rotate_fernet_key_command.py #59449
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.