Skip to content

Remove remaining session query usages#62758

Merged
potiuk merged 8 commits intoapache:mainfrom
kunaljubce:45461-remove-remaining-session-query-usages
Mar 10, 2026
Merged

Remove remaining session query usages#62758
potiuk merged 8 commits intoapache:mainfrom
kunaljubce:45461-remove-remaining-session-query-usages

Conversation

@kunaljubce
Copy link
Contributor

@kunaljubce kunaljubce commented Mar 3, 2026 •
edited
Loading

relates to: #45461

Details
As part of 45461, as far as I understand, only a last couple of session.query instances were left behind:

→ grep -R "session.query"
Binary file ./generated/_doctrees/docs/apache-airflow-providers-airbyte/environment.pickle matches
./providers/teradata/docs/connections/teradata.rst:QueryBand can be specified using extra connection configuration ... (Multiple instances of 'mock_session.query_tag' matches like the one below)
./providers/snowflake/tests/unit/snowflake/decorators/test_snowpark.py:            return session.query_tag
./providers/standard/tests/unit/standard/utils/test_skipmixin.py:            assert not session.query.called # CHANGE 1
./airflow-core/tests/unit/models/test_deadline.py:            mock_session.query.assert_not_called()         # CHANGE 2
./scripts/ci/prek/prevent_deprecated_sqlalchemy_usage.py:def check_session_query(mod: ast.Module, file_path: str) -> bool:
./scripts/ci/prek/prevent_deprecated_sqlalchemy_usage.py:        errors = check_session_query(ast_module, file_path)

These instances have been replaced with session.execute and session.scalars, dependending on their corresponding usages in the source code. Also fixed a potential dangling import issue I ran into with the prek pre-push hook:

Run mypy for providers...........................................................Failed
- hook id: mypy-providers
- exit code: 1

  Docker container engine detected.

  You are skipping the image upgrade check, but the image needs an upgrade. This 
  might lead to out-dated results of the check!
  Consider running `breeze ci-image build --python 3.10` at earliest convenience!


  Using 'uv' to install Airflow


  Using airflow version from current sources

  providers/standard/tests/unit/standard/utils/test_skipmixin.py:29: error:
  Module "airflow.utils" has no attribute "timezone"  [attr-defined]
      from airflow.utils import timezone
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Found 1 error in 1 file (checked 1 source file)

  Error 1 returned
  If you see strange stacktraces above, and can't reproduce it, please run this command and try again:

  breeze ci-image build --python 3.10

  You can also run `breeze down --cleanup-mypy-cache` to clean up the cache used.
Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)
  • Read the 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) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

kunaljubce
kunaljubce commented Mar 3, 2026
View reviewed changes
@Prab-27
Copy link
Contributor

Prab-27 commented Mar 4, 2026

Thanks @kunaljubce !! Good catch !!

Will work on this prek hooks again to see what we can do to catch this types of patterns !

@kunaljubce kunaljubce marked this pull request as ready for review March 4, 2026 08:48
@kunaljubce
Copy link
Contributor Author

kunaljubce commented Mar 4, 2026

Thanks @kunaljubce !! Good catch !!

Will work on this prek hooks again to see what we can do to catch this types of patterns !

Sure @Prab-27, please lemme know if you're occupied with other tasks. I can pick it up as well.

@kunaljubce kunaljubce mentioned this pull request Mar 4, 2026
Closed
2 tasks
Prab-27
Prab-27 approved these changes Mar 9, 2026
View reviewed changes
Copy link
Contributor

@Prab-27 Prab-27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !!

@Prab-27
Copy link
Contributor

Prab-27 commented Mar 9, 2026 •
edited
Loading

@kunaljubce Thanks !! Could you please share your thoughts

@ephraimbuddy since you created that issue and we are working on it :) It is good to check each sides - Could you please confirm weather any deprecated SQLAlchemy query left in our code bases ? and how to find it ?

All Done (including this only )! I have checked from my end

But @potiuk I'd like to confirm that this .venev files are packages that we use them so we don't need to remove deprecated query from it - ./.venv/lib/python3.10/site-packages/sqlalchemy/sql/elements.py like this types of files

@potiuk potiuk changed the title 45461 remove remaining session query usages Remove remaining session query usages Mar 10, 2026
@potiuk potiuk merged commit 7606f82 into apache:main Mar 10, 2026
92 checks passed
dominikhei pushed a commit to dominikhei/airflow that referenced this pull request Mar 11, 2026
@kunaljubce @dominikhei
Remove remaining session query usages (apache#62758)
99ef444 
* Remove remaining session.query usages

* Remove remaining session.query usages

* Change as per review comment

* Minor comment update in compat.sdk
Pyasma pushed a commit to Pyasma/airflow that referenced this pull request Mar 13, 2026
@kunaljubce @Pyasma
Remove remaining session query usages (apache#62758)
5f4fc63 
* Remove remaining session.query usages

* Remove remaining session.query usages

* Change as per review comment

* Minor comment update in compat.sdk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@uranusjr uranusjr uranusjr left review comments

@potiuk potiuk potiuk approved these changes

+1 more reviewer

@Prab-27 Prab-27 Prab-27 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:deadline-alerts AIP-86 (former AIP-57) area:providers provider:standard

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kunaljubce @Prab-27 @potiuk @uranusjr