Skip to content

Fix: Handle SQLAlchemy_URI for Vertica Provider#55928

Merged
jason810496 merged 6 commits into
apache:mainfrom
Pyasma:fix/vertica_sqlalchemy_uri
Sep 27, 2025
Merged

Fix: Handle SQLAlchemy_URI for Vertica Provider#55928
jason810496 merged 6 commits into
apache:mainfrom
Pyasma:fix/vertica_sqlalchemy_uri

Conversation

@Pyasma

@Pyasma Pyasma commented Sep 21, 2025

Copy link
Copy Markdown
Contributor

Summary
This PR resolves a issue where the VerticaHook in the apache-airflow-providers-vertica package failed to correctly construct a SQLAlchemy connection URI. The previous implementation, which relied on the default DbApiHook.get_uri() method, was unable to properly handle complex connection parameters, especially those provided in the extra field of the Airflow Connection.

This change introduces a new sqlalchemy_url property within the VerticaHook. This property now correctly parses all connection attributes, including the host, port, login, password, schema (database), and most importantly, any key-value pairs from the extra field. The get_uri() method is updated to return a valid string representation of this URL object, ensuring that the hook can reliably connect to Vertica databases using a correctly formed SQLAlchemy URI.

Related Issues
Related #38195 (Specifically, the Vertica portion of this issue)

Checklist

  • I have rebased my branch to the latest main branch.
  • I have run prek and all checks passed.

@boring-cyborg

boring-cyborg Bot commented Sep 21, 2025

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

Comment thread airflow-core/newsfragments/38195.bugfix.rst Outdated
@eladkal
eladkal requested a review from jason810496 September 21, 2025 07:22
Comment thread providers/vertica/src/airflow/providers/vertica/hooks/vertica.py Outdated
Comment thread providers/vertica/src/airflow/providers/vertica/hooks/vertica.py Outdated
@eladkal
eladkal requested a review from Lee-W September 22, 2025 11:44

@Lee-W Lee-W left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

mostly looks good. a few nitpicks

Comment thread providers/vertica/tests/unit/vertica/hooks/test_vertica_sql.py Outdated
Comment thread providers/vertica/tests/unit/vertica/hooks/test_vertica_sql.py
Comment thread providers/vertica/tests/unit/vertica/hooks/test_vertica_sql.py Outdated
@Pyasma

Pyasma commented Sep 24, 2025

Copy link
Copy Markdown
Contributor Author

Hi @jason810496, just following up on this PR. Please let me know if you have any questions or if there's anything else you need from me. Thanks!

@jason810496 jason810496 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi Piyush, I saw your message on Slack.
Thanks for the reminder!

It seems there are still some comments from Wei that have not yet been resolved.
LGTM after addressing those comments, and please don't hesitate to resolve the comments yourself.

Thanks!

@Pyasma

Pyasma commented Sep 26, 2025

Copy link
Copy Markdown
Contributor Author

Sure i will do that.

@jason810496
jason810496 merged commit fb09d46 into apache:main Sep 27, 2025
76 checks passed
@boring-cyborg

boring-cyborg Bot commented Sep 27, 2025

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

dabla pushed a commit to dabla/airflow that referenced this pull request Oct 12, 2025
* fix: vertica sqlalchemy uri

* Fix: add newline at end of 38195.bugfix.rst to pass pre-commit hooks

* Fix: Deleted the bugfix

* Added the annotations

* adjusting the format for mocks

* Changed the Mock Format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants