You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/11/02 19:11:37 UTC
[GitHub] [airflow] codekronik opened a new issue, #27474: Use full postgresql URI connection prefix.
codekronik opened a new issue, #27474:
URL: https://github.com/apache/airflow/issues/27474
### 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?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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: commits-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
bdsoha commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301164387
This has already been implemented in the [postgres hook](https://github.com/apache/airflow/blob/main/airflow/providers/postgres/CHANGELOG.rst#400).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] andersbogsnes commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
andersbogsnes commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1309179318
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?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301179810
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 https://github.com/apache/airflow/pull/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.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] andersbogsnes commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
andersbogsnes commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1318148843
@potiuk Sure - I'll give it a go 👍
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1303518363
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?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] codekronik commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
codekronik commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301237944
@potiuk I have the latest airflow, with latest packages.
I have tried rebuilding the image. I have tried refreshing the requirements, checked 3 times that I have the latest package version. :/
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301798831
And what is the connection you have defined for postgres that returns the URI and how you defined 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)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] codekronik commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
codekronik commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301175411
Hm. Odd I'm using Airflow 2.4.2 and its still returning old prefix.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301180153
Closing. We can reopen if there are new evidences.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301795413
Which postgres provider do you have ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] codekronik commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
codekronik commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1302756004
@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()`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1317754922
> 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 ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #27474: Use full postgresql URI connection prefix.
URL: https://github.com/apache/airflow/issues/27474
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #27474: Use full postgresql URI connection prefix.
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #27474:
URL: https://github.com/apache/airflow/issues/27474#issuecomment-1301097054
Thanks for opening your first issue here! Be sure to follow the issue template!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org