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 2021/03/30 14:02:50 UTC
[GitHub] [airflow] cccs-cat001 opened a new pull request #15089: Added azure postgres connection url format
cccs-cat001 opened a new pull request #15089:
URL: https://github.com/apache/airflow/pull/15089
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Adds back azure postgres connection info that got broke in the following commit: af5eb55943821cabe68c63f9e3287f828eede06c
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-813371373
I looked at it again, and this PR proposal is actually wrong. The URI you are trying to use is wrong.
We actually even have a detailed description about this in http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#handling-of-special-characters-in-connection-params and we even described how to generate the connection URI properly: http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#generating-connection-uri
You are not supposed to have `@` character in the user part of the URI. This is strictly forbidden in the https://tools.ietf.org/html/rfc3986 and if we allow that, it leads to a number of ambiguities.
We MUST strictly follow the specification rather than bypass it.
Read the "Reserved Characters" part of the specification. Those are the types of characters we have:
```
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="
```
Follow it further and you will see that while the user information can contain `sub-delims`, it
MUST NOT contain `gen-delims` characters. If they are needed, they must be percent-encoded:
```
userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
```
This means that in your case, if you want to use user name containing the `@` character (which is part of `gen-delims`) you must percent-encode it. Your user-name should look like this:
```
YourUserName%40YourDatabaseName:
```
I am closing that one as invalid - but it would be great @cccs-cat001 if you verify that the percent-encoding helps to address your problem.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-813371373
I looked at it again, and this PR proposal is actually wrong. The URI you are trying to use is wrong.
We actually even have a detailed description about this in http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#handling-of-special-characters-in-connection-params and we even described how to generate the connection URI properly: http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#generating-connection-uri
You are not supposed to have `@` character in the user part of the URI. This is strictly forbidden in the https://tools.ietf.org/html/rfc3986 and if we allow that, it leads to a number of ambiguities.
We MUST strictly follow the specification rather than bypass it.
Read the "Reserved Characters" part of the specification. Those are the types of characters we have:
```
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="
```
Follow it further and you will see that while the user information can contain `sub-delims`, it
MUST NOT contain `gen-delims` characters. If they are needed, they must be percent-encoded:
```
userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
```
This means that in your case, if you want to use user name containing the `@` character (which is part of `gen-delims`) you must percent-encode it. Your user info part should look like this:
```
YourUserName%40YourDatabaseName:
```
I am closing that one as invalid - but it would be great @cccs-cat001 if you verify that the percent-encoding helps to address your problem.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-810661562
What version of Airflow are you using? This code should not run on Airflow 1.10.14 and later.
https://github.com/apache/airflow/blob/349ccf8861337c1151948d938d30dfbad386fac0/scripts/in_container/prod/entrypoint_prod.sh#L198
Maybe we should add support for the `airflow checkdb` command to support older Airflow versions as well?
http://airflow.apache.org/docs/apache-airflow/1.10.13/cli-ref.html#checkdb
This command was added in Airflow 1.10.8, so it's a bit of a mature 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] cccs-cat001 edited a comment on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
cccs-cat001 edited a comment on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-810970367
>
>
> What version of Airflow are you using? This code should not run on Airflow 1.10.14 and later.
>
> https://github.com/apache/airflow/blob/349ccf8861337c1151948d938d30dfbad386fac0/scripts/in_container/prod/entrypoint_prod.sh#L198
>
> Maybe we should add support for the `airflow checkdb` command to support older Airflow versions as well?
> http://airflow.apache.org/docs/apache-airflow/1.10.13/cli-ref.html#checkdb
> This command was added in Airflow 1.10.8, so it's a bit of a mature version.
>
> I would like to eliminate the source of the problem, so that it would not be necessary to make further improvements to this regular expression.
I'm using airflow 2.0.1. without this change, the entrypoint fails saying BACKEND is not defined.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-813371373
I looked at it again, and this PR proposal is actually wrong. The URI you are trying to use is wrong.
We actually even have a detailed description about this in http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#handling-of-special-characters-in-connection-params and we even described how to generate the connection URI properly: http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#generating-connection-uri
You are not supposed to have `@` character in the user part of the URI. This is strictly forbidden in the https://tools.ietf.org/html/rfc3986 and if we allow that, it leads to a number of ambiguities.
We MUST strictly follow the specification rather than bypass it.
Read the "Reserved Characters" part of the specification. Those are the `gen-delims` charatcters :
```
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="
```
Follow it further and you will see that while the user information can contain sub-delims, it
MUST NOT contain gen-delims characters. If they are needed, they must be percent-encoded:
```
userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
```
This means that in your case, if you want to use user name containing the `@` character (which is part of `gen-delims`) you must percent-encode it. Your user-name should look like this:
```
YourUserName%40YourDatabaseName:
```
I am closing that one as invalid - but it would be great @cccs-cat001 if you verify that the percent-encoding helps to address your problem.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] cccs-cat001 commented on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
cccs-cat001 commented on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-810990262
Yeah I just ran with that version of the file. The `airflow db check` doesn't run, it goes straight to the wait_for_connection.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] cccs-cat001 commented on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
cccs-cat001 commented on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-810980008
in the airflow:2.0.1-python3.8 image I'm using, this line you're quoting doesn't exist, it always runs the wait_for_connection function
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-813371373
I looked at it again, and this PR proposal is actually wrong. The URI you are trying to use is not correct according to URI specfication.
We actually even have a detailed description about this in http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#handling-of-special-characters-in-connection-params and we even described how to generate the connection URI properly: http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#generating-connection-uri
You are not supposed to have `@` character in the user part of the URI. This is strictly forbidden in the https://tools.ietf.org/html/rfc3986 and if we allow that, it leads to a number of ambiguities.
We MUST strictly follow the specification rather than bypass it.
Read the "Reserved Characters" part of the specification. Those are the types of characters we have:
```
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="
```
Follow it further and you will see that while the user information can contain `sub-delims`, it
MUST NOT contain `gen-delims` characters. If they are needed, they must be percent-encoded:
```
userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
```
This means that in your case, if you want to use user name containing the `@` character (which is part of `gen-delims`) you must percent-encode it. Your user info part should look like this:
```
YourUserName%40YourDatabaseName:
```
I am closing that one as invalid - but it would be great @cccs-cat001 if you verify that the percent-encoding helps to address your problem.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #15089:
URL: https://github.com/apache/airflow/pull/15089
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] cccs-cat001 commented on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
cccs-cat001 commented on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-810970367
>
>
> What version of Airflow are you using? This code should not run on Airflow 1.10.14 and later.
>
> https://github.com/apache/airflow/blob/349ccf8861337c1151948d938d30dfbad386fac0/scripts/in_container/prod/entrypoint_prod.sh#L198
>
> Maybe we should add support for the `airflow checkdb` command to support older Airflow versions as well?
> http://airflow.apache.org/docs/apache-airflow/1.10.13/cli-ref.html#checkdb
> This command was added in Airflow 1.10.8, so it's a bit of a mature version.
>
> I would like to eliminate the source of the problem, so that it would not be necessary to make further improvements to this regular expression.
I'm using airflow 2.0.1.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-810983509
@cccs-cat001 it is a new feature added by me. Can you copy this file to your image to check if this patch meets your requirements?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-810661562
What version of Airflow are you using? This code should not run on Airflow 1.10.14 and later.
https://github.com/apache/airflow/blob/349ccf8861337c1151948d938d30dfbad386fac0/scripts/in_container/prod/entrypoint_prod.sh#L198
Maybe we should add support for the `airflow checkdb` command to support older Airflow versions as well?
http://airflow.apache.org/docs/apache-airflow/1.10.13/cli-ref.html#checkdb
This command was added in Airflow 1.10.8, so it's a bit of a mature version.
I would like to eliminate the source of the problem, so that it would not be necessary to make further improvements to this regular expression.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #15089: Added azure postgres connection url format
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15089:
URL: https://github.com/apache/airflow/pull/15089#issuecomment-813371373
I looked at it again, and this PR proposal is actually wrong. The URI you are trying to use is wrong.
We actually even have a detailed description about this in http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#handling-of-special-characters-in-connection-params and we even described how to generate the connection URI properly: http://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html?highlight=uri#generating-connection-uri
You are not supposed to have `@` character in the user part of the URI. This is strictly forbidden in the https://tools.ietf.org/html/rfc3986 and if we allow that, it leads to a number of ambiguities.
We MUST strictly follow the specification rather than bypass it.
Read the "Reserved Characters" part of the specification. Those are the types of characters we have:
```
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="
```
Follow it further and you will see that while the user information can contain sub-delims, it
MUST NOT contain gen-delims characters. If they are needed, they must be percent-encoded:
```
userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
```
This means that in your case, if you want to use user name containing the `@` character (which is part of `gen-delims`) you must percent-encode it. Your user-name should look like this:
```
YourUserName%40YourDatabaseName:
```
I am closing that one as invalid - but it would be great @cccs-cat001 if you verify that the percent-encoding helps to address your problem.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org