Skip to content

Use full postgresql URI connection prefix. #27474

Closed

Description

@codekronik

Apache Airflow version

2.4.2

What happened

This creates a deprecated URI string. I've added a new connection, using get_uri() returns a URI with postgres:// instead of postgressql:// so it's incompatible with a few libs. Should this be updated to return valid format URI ?

What you think should happen instead

The URI should return URI with postgresql:// prefix.

How to reproduce

Add a new postgres connection, pull up the Connection by ID in DAG and try using it directly with SQLAlchemy

Operating System

Debian

Versions of Apache Airflow Providers

No response

Deployment

Docker-Compose

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

Activity

  1. boring-cyborg commented on Nov 2, 2022

    @boring-cyborg

    Thanks for opening your first issue here! Be sure to follow the issue template!

  2. bdsoha commented on Nov 2, 2022

    @bdsoha
    Contributor

    This has already been implemented in the postgres hook, since version 4.0.0 of the provider.

  3. codekronik commented on Nov 2, 2022

    @codekronik
    Author

    Hm. Odd I'm using Airflow 2.4.2 and its still returning old prefix with latest packages.

  4. potiuk commented on Nov 2, 2022

    @potiuk
    Member

    You must have wrong Provider version: https://airflow.apache.org/docs/apache-airflow-providers-postgres/stable/index.html#id12 (postgres provider 4.0.0 has this fix implemented #21205. I guess you have not installed/upgraded to the new version of airflow with constraints (https://airflow.apache.org/docs/apache-airflow/stable/installation/installing-from-pypi.html) and you have an old version of Postgres provider.

  5. potiuk commented on Nov 2, 2022

    @potiuk
    Member

    Closing. We can reopen if there are new evidences.

  6. codekronik commented on Nov 2, 2022

    @codekronik
    Author

    @potiuk I have the latest airflow, with latest packages.

    Screenshot 2022-11-02 215436

    Screenshot 2022-11-02 215608

    I have tried rebuilding the image. I have tried refreshing the requirements, checked 3 times that I have the latest package version. :/

  7. potiuk commented on Nov 3, 2022

    @potiuk
    Member

    Which postgres provider do you have ?

  8. potiuk commented on Nov 3, 2022

    @potiuk
    Member

    And what is the connection you have defined for postgres that returns the URI and how you defined it? And the exact code you use to retrieve it ?

    Generally the problem is solved, so it is like some problem with your configuration/ways of defining or using the connections If you do not provide reproducible steps how to get to the same results it will be difficult to help to solve your problem (or find a flaw in Airflow which might also be the case but I think it is unlikely)

  9. codekronik commented on Nov 3, 2022

    @codekronik
    Author

    @potiuk Ok the issue appears because this has not been updated in the Connection model. Basically, if you do BaseHook.get_connection(conn_id).get_uri() it works as expected if you do PostgresHook.get_hook(conn_id).get_uri()

  10. potiuk commented on Nov 4, 2022

    @potiuk
    Member

    You still have not explained how did you define your connection - What is the connection type you have in your connection?
    You might have be completely mislead - PostgressHook.get_hook() is NOT a PostgresHook method - it is the "BaseHook" static method - and it will only return PostgresHook if you pass it the connection, that it will recognise as Postgres - this is likely the reason - likely "PostgresHook.get_hook()" will actually return non-PostgresHook but something else.

    Can you please explain how your connection_id is defined and what type it has ? And what is the hook returned by get_hook method?

  11. andersbogsnes commented on Nov 9, 2022

    @andersbogsnes

    Someone from my team ran into a similar problem. I believe the problem stems from this line: https://github.com/apache/airflow/blob/main/airflow/models/connection.py#L215.

    If you use BaseHook.get_connection() passing in a "Postgres" type connection, the conn_type will be "postgres": https://github.com/apache/airflow/blob/main/airflow/providers/postgres/hooks/postgres.py#L66, meaning the final uri will start with postgres instead of postgresql.

    The fix as I see it, is to use either BaseHook.get_hook(conn_id).get_uri() if you want a generic solution or to use the PostgresHook directly but I guess the question is whether or not BaseHook.get_uri() is supposed to render the uri correctly in all cases?

  12. potiuk commented on Nov 16, 2022

    @potiuk
    Member

    Someone from my team ran into a similar problem. I believe the problem stems from this line: https://github.com/apache/airflow/blob/main/airflow/models/connection.py#L215.

    If you use BaseHook.get_connection() passing in a "Postgres" type connection, the conn_type will be "postgres": https://github.com/apache/airflow/blob/main/airflow/providers/postgres/hooks/postgres.py#L66, meaning the final uri will start with postgres instead of postgresql.

    The fix as I see it, is to use either BaseHook.get_hook(conn_id).get_uri() if you want a generic solution or to use the PostgresHook directly but I guess the question is whether or not BaseHook.get_uri() is supposed to render the uri correctly in all cases?

    Would you like to make a PR to fix it @andersbogsnes ?

  13. andersbogsnes commented on Nov 17, 2022

    @andersbogsnes

    @potiuk Sure - I'll give it a go 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions