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 2023/01/11 09:58:11 UTC

[GitHub] [airflow] aru-trackunit opened a new pull request, #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

aru-trackunit opened a new pull request, #28852:
URL: https://github.com/apache/airflow/pull/28852

   closes: https://github.com/apache/airflow/issues/28766
   
   I believe that this is a bug in the `get_conn_uri` method but I am not sure whether this is the best approach towards it.
   Happy to make only documentation changes so everyone else doesn't face the same 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] aru-trackunit commented on pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1378590272

   @Taragolis ok, I will revert my changes and update only docs.
   
   As a side note: I am not a python developer and I find contributing pretty difficult. Especially setting up the environment and running tests.
   When setting up a local environment I went into issues like missing ``pygraphviz``, then after executing tests using breeze it was missing some packages. Then `zshrc` script couldn't find `compdef`. Stackoverflow again to the rescue. Finally when I got them to work then some tests failed even when I haven't done any code changes.
   
   When running `breeze testing tests` it took my local aws account and tried to execute something which obviously failed
   ```
   botocore.exceptions.ClientError: An error occurred (AccessDeniedException) when calling the GetParameter operation: User: arn:aws:sts::----:assumed-role/---- is not authorized to perform: ssm:GetParameter on resource: arn:aws:ssm:us-east-1: :parameter/example_step_functions because no identity-based policy allows the ssm:GetParameter action
   ```
   
   It's hard to contribute when you are not very familiar with python. I have a feeling that there is tons of documentation to read, just to make a few line changes. Probably I know to little about the ecosystem to propose solution, but the problems I have faced discourage me from contributing.


-- 
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] uranusjr commented on a diff in pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1070870799


##########
docs/apache-airflow-providers-amazon/connections/aws.rst:
##########
@@ -146,6 +146,18 @@ Snippet to create Connection and convert to URI
     os.environ[env_key] = conn_uri
     print(conn.test_connection())
 
+
+  .. warning:: When using airflow CLI there might be a need to add ``@`` when:

Review Comment:
   ```suggestion
     .. warning:: When using the Airflow CLI, a ``@`` may need to be added when:
   ```



-- 
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] vincbeck commented on pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
vincbeck commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1380556076

   Yes. System tests which need external values ([example here](https://github.com/apache/airflow/blob/main/tests/system/providers/amazon/aws/example_batch.py#L47)) try to fetch these values in [SSM parameter store](https://docs.aws.amazon.com/systems-manager/latest/userguide/systems-manager-parameter-store.html) unless they are defined as environment variable. [See logic here](https://github.com/apache/airflow/blob/f1eb2f1af42c537f7c49a891f238083fd5d9e762/tests/system/providers/amazon/aws/utils/__init__.py#L216). It seems like we are already handling the case when no credentials is provided ([here](https://github.com/apache/airflow/blob/f1eb2f1af42c537f7c49a891f238083fd5d9e762/tests/system/providers/amazon/aws/utils/__init__.py#L102)). We might want to update this logic to handle the case when the credentials do not have enough permissions to read parameters from SSM


-- 
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] Taragolis commented on a diff in pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1068355847


##########
docs/apache-airflow-providers-amazon/connections/aws.rst:
##########
@@ -148,6 +148,18 @@ Snippet to create Connection and convert to URI
     os.environ[env_key] = conn_uri
     print(conn.test_connection())
 
+
+  .. warning:: When using airflow CLI there might be a need to add ``@`` when:
+
+    - login
+    - password,
+    - host
+    - port
+
+    are not given see example below. This is a known airflow limitation.
+
+    ``airflow connections add aws_conn --conn-uri aws://@/?egion_name=eu-west-1``

Review Comment:
   ```suggestion
       ``airflow connections add aws_conn --conn-uri aws://@/?region_name=eu-west-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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] aru-trackunit commented on a diff in pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1070872547


##########
docs/apache-airflow-providers-amazon/connections/aws.rst:
##########
@@ -146,6 +146,18 @@ Snippet to create Connection and convert to URI
     os.environ[env_key] = conn_uri
     print(conn.test_connection())
 
+
+  .. warning:: When using airflow CLI there might be a need to add ``@`` when:
+
+    - login
+    - password,
+    - host
+    - port

Review Comment:
   ๐Ÿ‘ good find



-- 
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 merged pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
potiuk merged PR #28852:
URL: https://github.com/apache/airflow/pull/28852


-- 
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] aru-trackunit commented on a diff in pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1066799566


##########
tests/always/test_connection.py:
##########
@@ -275,13 +275,13 @@ def test_connection_extra_with_encryption_rotate_fernet_key(self):
             description="no schema",
         ),
         UriTestCaseConfig(
-            test_conn_uri="google-cloud-platform://?key_path=%2Fkeys%2Fkey.json&scope="
+            test_conn_uri="google-cloud-platform://@/?key_path=%2Fkeys%2Fkey.json&scope="
             "https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fcloud-platform&project=airflow",
             test_conn_attributes=dict(
                 conn_type="google_cloud_platform",
                 host="",
                 schema="",
-                login=None,
+                login="",

Review Comment:
   Can this change impact unexpected behaviours in the other part of the app? I am reffering that instead of returning None it returns an empty string (not that I modified the test)



-- 
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] uranusjr commented on pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1383546841

   If Iโ€™m reading the linked issue correctly, this is fixed in main, right? In that case we should only add this to older versions. Or is there something I misunderstand?


-- 
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] Taragolis commented on pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1380457475

   > Sure!
   > 
   > 1. I am storing them in .aws/config file as profiles with default profile enabled.
   > 2. Mac M1
   > 3. Yes:
   > 
   > ```
   > ERROR tests/system/providers/amazon/aws/example_athena.py - botocore.exceptio...
   > ERROR tests/system/providers/amazon/aws/example_batch.py - botocore.exception...
   > ERROR tests/system/providers/amazon/aws/example_cloudformation.py - botocore....
   > ERROR tests/system/providers/amazon/aws/example_datasync.py - botocore.except...
   > ERROR tests/system/providers/amazon/aws/example_dms.py - botocore.exceptions....
   > ```
   > 
   > I am open to give you more input if needed
   
   @ferruzzi @vincbeck @o-nikolas guys, I'm not familiar with system tests is it expected that system tests tried to obtain credentials?
   


-- 
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] aru-trackunit commented on a diff in pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1066799566


##########
tests/always/test_connection.py:
##########
@@ -275,13 +275,13 @@ def test_connection_extra_with_encryption_rotate_fernet_key(self):
             description="no schema",
         ),
         UriTestCaseConfig(
-            test_conn_uri="google-cloud-platform://?key_path=%2Fkeys%2Fkey.json&scope="
+            test_conn_uri="google-cloud-platform://@/?key_path=%2Fkeys%2Fkey.json&scope="
             "https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fcloud-platform&project=airflow",
             test_conn_attributes=dict(
                 conn_type="google_cloud_platform",
                 host="",
                 schema="",
-                login=None,
+                login="",

Review Comment:
   Can this change cause unexpected behaviours in the other part of the app? I am reffering to returning empty string instead of None (not that I modified the test)



-- 
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 pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1384362745

   Awesome work, congrats on your first merged pull request!
   


-- 
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 pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1378498847

   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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-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](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) 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](https://www.apache.org/foundation/policies/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]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
   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
   


-- 
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] aru-trackunit commented on pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1380183451

   Sure!
   1. I am storing them in .aws/config file as profiles with default profile enabled.
   2. Mac M1
   3. Yes:
   ```
   ERROR tests/system/providers/amazon/aws/example_athena.py - botocore.exceptio...
   ERROR tests/system/providers/amazon/aws/example_batch.py - botocore.exception...
   ERROR tests/system/providers/amazon/aws/example_cloudformation.py - botocore....
   ERROR tests/system/providers/amazon/aws/example_datasync.py - botocore.except...
   ERROR tests/system/providers/amazon/aws/example_dms.py - botocore.exceptions....
   ```
   
   I am open to give you more input if needed


-- 
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] uranusjr commented on a diff in pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1070871016


##########
docs/apache-airflow-providers-amazon/connections/aws.rst:
##########
@@ -146,6 +146,18 @@ Snippet to create Connection and convert to URI
     os.environ[env_key] = conn_uri
     print(conn.test_connection())
 
+
+  .. warning:: When using airflow CLI there might be a need to add ``@`` when:
+
+    - login
+    - password,
+    - host
+    - port

Review Comment:
   Why does only the password have a comma attached? Does this mean something I donโ€™t get?



-- 
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] Taragolis commented on pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1383839268

   @aru-trackunit Yeah this PR still relevant, because https://github.com/apache/airflow/pull/28922 in main and potentially would include in 2.5.2 (or later), however latest version of Amazon Provider has requirement AIrflow 2.3+.
   
   So would be nice to have this info for users who will use old version of Airflow


-- 
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] Taragolis commented on pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1378632696

   > @Taragolis ok, I will revert my changes and update only docs.
   
   Contribution to documentation it still important part, personally I think that documentation part more important rather than code.
   
   > As a side note: I am not a python developer and I find contributing pretty difficult. Especially setting up the environment and running tests.
   
   No problem if you are not feel comfortable someone could pick up part for solve actual issue. I have a plan to work on it in the next couple days if someone not pick it up.
   
   The documentation part is still important. Just one suggestion it would be nice also add info about issue with adding connection by URI in cli into the [Amazon Web Services Connection page](https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/connections/aws.html).
   
   We mentioned here that user could use `aws://` in case of default `boto3` credentials strategy (env variables, EC2/ECS metadata and etc). And this connection URI is valid in all places except cli.
   
   ```console
   
   โฏ airflow connections add sample-aws-conn --conn-uri "aws://"
   The URI provided to --conn-uri is invalid: aws://
   ```
   
   I could say it is not a common way to add connection by cli as URI because nobody mentioned it before and users could add it by parts or since airflow 2.3 as json ([doc](https://airflow.apache.org/docs/apache-airflow/stable/cli-and-env-variables-ref.html#add)) however it nice that we know that we have this bug.
   
   > When running `breeze testing tests` it took my local aws account and tried to execute something which obviously failed
   >
   > ```botocore.exceptions.ClientError: An error occurred (AccessDeniedException) when calling the GetParameter operation: User: arn:aws:sts::----:assumed-role/---- is not authorized to perform: ssm:GetParameter on resource: arn:aws:ssm:us-east-1: :parameter/example_step_functions because no identity-based policy allows the ssm:GetParameter action```
   
   Interesting, I thought we solve issue when `amazon-provider` tests obtain actual credentials. Could you provide a bit more info here?
   1. How you store your credentials? Environment variables, profile and etc.
   2. Where you try to run this test? Laptop / PC or in EC2?


-- 
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] aru-trackunit commented on a diff in pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1066799566


##########
tests/always/test_connection.py:
##########
@@ -275,13 +275,13 @@ def test_connection_extra_with_encryption_rotate_fernet_key(self):
             description="no schema",
         ),
         UriTestCaseConfig(
-            test_conn_uri="google-cloud-platform://?key_path=%2Fkeys%2Fkey.json&scope="
+            test_conn_uri="google-cloud-platform://@/?key_path=%2Fkeys%2Fkey.json&scope="
             "https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fcloud-platform&project=airflow",
             test_conn_attributes=dict(
                 conn_type="google_cloud_platform",
                 host="",
                 schema="",
-                login=None,
+                login="",

Review Comment:
   Can this change impact unexpected behaviours in the other part of the app?



-- 
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] Taragolis commented on pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1378532814

   @aru-trackunit the actual problem in cli command, not in `Connection` object.
   
   We should fix this function:
   https://github.com/apache/airflow/blob/45dd0c484e16ff56800cc9c047f56b4a909d2d0d/airflow/cli/commands/connection_command.py#L134-L137


-- 
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] aru-trackunit commented on a diff in pull request #28852: Fixing get_conn_uri method - adding ``@`` when login, password, host, port is not given

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1066798202


##########
tests/always/test_connection.py:
##########
@@ -275,13 +275,13 @@ def test_connection_extra_with_encryption_rotate_fernet_key(self):
             description="no schema",
         ),
         UriTestCaseConfig(
-            test_conn_uri="google-cloud-platform://?key_path=%2Fkeys%2Fkey.json&scope="
+            test_conn_uri="google-cloud-platform://@/?key_path=%2Fkeys%2Fkey.json&scope="

Review Comment:
   Ideally I wouldn't update tests, but as far as I understand it doesn't comply with the CLI contract.



-- 
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] XD-DENG commented on a diff in pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on code in PR #28852:
URL: https://github.com/apache/airflow/pull/28852#discussion_r1070382236


##########
docs/apache-airflow-providers-amazon/connections/aws.rst:
##########
@@ -148,6 +148,18 @@ Snippet to create Connection and convert to URI
     os.environ[env_key] = conn_uri
     print(conn.test_connection())
 
+
+  .. warning:: When using airflow CLI there might be a need to add ``@`` when:
+
+    - login
+    - password,
+    - host
+    - port
+
+    are not given see example below. This is a known airflow limitation.

Review Comment:
   Is there a comma or something missing here between `are not given` and  `see example below`?



##########
docs/apache-airflow-providers-amazon/logging/s3-task-handler.rst:
##########
@@ -113,17 +113,25 @@ We are  using the existing ``serviceAccount`` hence ``create: false`` with exist
         delete_worker_pods: 'False'
         encrypt_s3_logs: 'True'
 
-Step3: Create Amazon Web Services connection in Airflow Web UI
+Step3: Create Amazon Web Services connection
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 With the above configurations, Webserver and Worker Pods can access Amazon S3 bucket and write logs without using any Access Key and Secret Key or Instance profile credentials.
 
-The final step to create connections under Airflow UI before executing the DAGs.
+- Using Airflow Web UI
 
-* Login to Airflow Web UI with ``admin`` credentials and Navigate to ``Admin -> Connections``
-* Create connection for ``Amazon Web Services`` and select the options(Connection ID and Connection Type) as shown in the image.
-* Select the correct region where S3 bucket is created in ``Extra`` text box.
+  The final step to create connections under Airflow UI before executing the DAGs.
 
-.. image:: /img/aws-base-conn-airflow.png
+  * Login to Airflow Web UI with ``admin`` credentials and Navigate to ``Admin -> Connections``
+  * Create connection for ``Amazon Web Services`` and select the options(Connection ID and Connection Type) as shown in the image.

Review Comment:
   ```suggestion
     * Create connection for ``Amazon Web Services`` and select the options (Connection ID and Connection Type) as shown in the image.
   ```



-- 
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] aru-trackunit commented on pull request #28852: Add documentation about cli `add connection` and AWS connection URI

Posted by GitBox <gi...@apache.org>.
aru-trackunit commented on PR #28852:
URL: https://github.com/apache/airflow/pull/28852#issuecomment-1383525851

   @Taragolis I've seen you added another PR. Is the docs that I wrote still relevant or this PR should be closed?


-- 
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