You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "josh-fell (via GitHub)" <gi...@apache.org> on 2023/03/03 07:08:04 UTC
[GitHub] [airflow] josh-fell opened a new pull request, #29892: Add testing a connection via Airflow CLI
josh-fell opened a new pull request, #29892:
URL: https://github.com/apache/airflow/pull/29892
Closes: #29875
This PR introduces a new Airflow CLI command -- `airflow connections test <conn_id>` -- to test a single, predefined Airflow connection.
_Successful test_
<img width="521" alt="image" src="https://user-images.githubusercontent.com/48934154/222654162-998fbe8e-e1c0-4ec1-846e-896f34067480.png">
_Failed test_
<img width="528" alt="image" src="https://user-images.githubusercontent.com/48934154/222654246-043df163-a3e0-4d86-87fc-e780fe13572f.png">
_TODO:_
- [ ] Add tests (unironically)
--
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] josh-fell commented on a diff in pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1130018126
##########
docs/apache-airflow/howto/connection.rst:
##########
@@ -191,19 +191,23 @@ Passwords cannot be manipulated or read without the key. For information on conf
Testing Connections
^^^^^^^^^^^^^^^^^^^
-Airflow Web UI & API allows to test connections. The test connection feature can be used from
-:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page, or through calling
-:doc:`Connections REST API </stable-rest-api-ref/>`.
+Airflow Web UI, REST API, and CLI allow you to test connections. The test connection feature can be used from
+:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page in the UI, through calling
+:doc:`Connections REST API </stable-rest-api-ref/>`, or running the ``airflow connections test`` :ref:`CLI command <cli>`.
-To test a connection Airflow calls out the ``test_connection`` method from the associated hook class and reports the
-results of it. It may happen that the connection type does not have any associated hook or the hook doesn't have the
-``test_connection`` method implementation, in either case the error message will throw the proper error message.
+To test a connection, Airflow calls the ``test_connection`` method from the associated hook class and reports the
+results. It may happen that the connection type does not have any associated hook or the hook doesn't have the
+``test_connection`` method implementation, in either case an error message will be displayed or functionality
+will be disabled (if you are testing in the UI).
-One important point to note is that the connections will be tested from the webserver only, so this feature is
+One important point to note is that when testing in the Airflow UI, the test executes from the webserver only. This feature is
Review Comment:
@msumit Made some small tweaks and explicitly added warning/note directives so these statement stand out. Let me know what you think when you get a chance.
--
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] josh-fell commented on pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on PR #29892:
URL: https://github.com/apache/airflow/pull/29892#issuecomment-1456512829
@potiuk @eladkal It occurred to me that the docs should be updated include this test method too. Just pushed some docs updates. Would you mind taking another look whenever you get a chance please?
--
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 pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29892:
URL: https://github.com/apache/airflow/pull/29892#issuecomment-1454843666
:heart: it
--
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 #29892: Introduce testing a connection via Airflow CLI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29892:
URL: https://github.com/apache/airflow/pull/29892
--
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] msumit commented on a diff in pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "msumit (via GitHub)" <gi...@apache.org>.
msumit commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1127360874
##########
docs/apache-airflow/howto/connection.rst:
##########
@@ -191,19 +191,23 @@ Passwords cannot be manipulated or read without the key. For information on conf
Testing Connections
^^^^^^^^^^^^^^^^^^^
-Airflow Web UI & API allows to test connections. The test connection feature can be used from
-:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page, or through calling
-:doc:`Connections REST API </stable-rest-api-ref/>`.
+Airflow Web UI, REST API, and CLI allow you to test connections. The test connection feature can be used from
+:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page in the UI, through calling
+:doc:`Connections REST API </stable-rest-api-ref/>`, or running the ``airflow connections test`` :ref:`CLI command <cli>`.
-To test a connection Airflow calls out the ``test_connection`` method from the associated hook class and reports the
-results of it. It may happen that the connection type does not have any associated hook or the hook doesn't have the
-``test_connection`` method implementation, in either case the error message will throw the proper error message.
+To test a connection, Airflow calls the ``test_connection`` method from the associated hook class and reports the
+results. It may happen that the connection type does not have any associated hook or the hook doesn't have the
+``test_connection`` method implementation, in either case an error message will be displayed or functionality
+will be disabled (if you are testing in the UI).
-One important point to note is that the connections will be tested from the webserver only, so this feature is
+One important point to note is that when testing in the Airflow UI, the test executes from the webserver only. This feature is
Review Comment:
@josh-fell my bad, the modified wording abt webserver looks good.. yeah we should add a bit abt CLI as well..
--
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 #29892: Introduce testing a connection via Airflow CLI
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1129841395
##########
airflow/cli/commands/connection_command.py:
##########
@@ -325,3 +325,23 @@ def _import_helper(file_path: str, overwrite: bool) -> None:
session.merge(conn)
session.commit()
print(f"Imported connection {conn_id}")
+
+
+@suppress_logs_and_warning
+def connections_test(args) -> None:
+ """Test an Airflow connection."""
+ console = AirflowConsole()
+
+ try:
+ print(f"Retrieving connection: {args.conn_id!r}")
+ conn = BaseHook.get_connection(args.conn_id)
+ except AirflowNotFoundException:
+ console.print("[bold yellow]\nConnection not found.\n")
+ raise SystemExit()
Review Comment:
This should use a non-zero return code.
--
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] josh-fell commented on a diff in pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1130000369
##########
airflow/cli/commands/connection_command.py:
##########
@@ -325,3 +325,23 @@ def _import_helper(file_path: str, overwrite: bool) -> None:
session.merge(conn)
session.commit()
print(f"Imported connection {conn_id}")
+
+
+@suppress_logs_and_warning
+def connections_test(args) -> None:
+ """Test an Airflow connection."""
+ console = AirflowConsole()
+
+ try:
+ print(f"Retrieving connection: {args.conn_id!r}")
+ conn = BaseHook.get_connection(args.conn_id)
+ except AirflowNotFoundException:
+ console.print("[bold yellow]\nConnection not found.\n")
+ raise SystemExit()
Review Comment:
Another great point. Will update.
--
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] josh-fell commented on a diff in pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1129999900
##########
airflow/cli/commands/connection_command.py:
##########
@@ -325,3 +325,23 @@ def _import_helper(file_path: str, overwrite: bool) -> None:
session.merge(conn)
session.commit()
print(f"Imported connection {conn_id}")
+
+
+@suppress_logs_and_warning
+def connections_test(args) -> None:
+ """Test an Airflow connection."""
+ console = AirflowConsole()
+
+ try:
+ print(f"Retrieving connection: {args.conn_id!r}")
Review Comment:
Excellent point here; you're right. This should really be outside the try block.
--
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 #29892: Introduce testing a connection via Airflow CLI
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1129840781
##########
airflow/cli/commands/connection_command.py:
##########
@@ -325,3 +325,23 @@ def _import_helper(file_path: str, overwrite: bool) -> None:
session.merge(conn)
session.commit()
print(f"Imported connection {conn_id}")
+
+
+@suppress_logs_and_warning
+def connections_test(args) -> None:
+ """Test an Airflow connection."""
+ console = AirflowConsole()
+
+ try:
+ print(f"Retrieving connection: {args.conn_id!r}")
Review Comment:
Any reason this is not outside the try block?
--
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 #29892: Introduce testing a connection via Airflow CLI
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1129841395
##########
airflow/cli/commands/connection_command.py:
##########
@@ -325,3 +325,23 @@ def _import_helper(file_path: str, overwrite: bool) -> None:
session.merge(conn)
session.commit()
print(f"Imported connection {conn_id}")
+
+
+@suppress_logs_and_warning
+def connections_test(args) -> None:
+ """Test an Airflow connection."""
+ console = AirflowConsole()
+
+ try:
+ print(f"Retrieving connection: {args.conn_id!r}")
+ conn = BaseHook.get_connection(args.conn_id)
+ except AirflowNotFoundException:
+ console.print("[bold yellow]\nConnection not found.\n")
+ raise SystemExit()
Review Comment:
This should be `sys.exit()` _with a non-zero return code_.
--
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] josh-fell commented on a diff in pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1129999900
##########
airflow/cli/commands/connection_command.py:
##########
@@ -325,3 +325,23 @@ def _import_helper(file_path: str, overwrite: bool) -> None:
session.merge(conn)
session.commit()
print(f"Imported connection {conn_id}")
+
+
+@suppress_logs_and_warning
+def connections_test(args) -> None:
+ """Test an Airflow connection."""
+ console = AirflowConsole()
+
+ try:
+ print(f"Retrieving connection: {args.conn_id!r}")
Review Comment:
Excellent point here; you're right. It should really be outside the try block.
--
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 pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29892:
URL: https://github.com/apache/airflow/pull/29892#issuecomment-1458936598
I rebased to account for the new version Python + Pipx caching issue from yesterday.
--
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] josh-fell commented on a diff in pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1127240069
##########
docs/apache-airflow/howto/connection.rst:
##########
@@ -191,19 +191,23 @@ Passwords cannot be manipulated or read without the key. For information on conf
Testing Connections
^^^^^^^^^^^^^^^^^^^
-Airflow Web UI & API allows to test connections. The test connection feature can be used from
-:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page, or through calling
-:doc:`Connections REST API </stable-rest-api-ref/>`.
+Airflow Web UI, REST API, and CLI allow you to test connections. The test connection feature can be used from
+:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page in the UI, through calling
+:doc:`Connections REST API </stable-rest-api-ref/>`, or running the ``airflow connections test`` :ref:`CLI command <cli>`.
-To test a connection Airflow calls out the ``test_connection`` method from the associated hook class and reports the
-results of it. It may happen that the connection type does not have any associated hook or the hook doesn't have the
-``test_connection`` method implementation, in either case the error message will throw the proper error message.
+To test a connection, Airflow calls the ``test_connection`` method from the associated hook class and reports the
+results. It may happen that the connection type does not have any associated hook or the hook doesn't have the
+``test_connection`` method implementation, in either case an error message will be displayed or functionality
+will be disabled (if you are testing in the UI).
-One important point to note is that the connections will be tested from the webserver only, so this feature is
+One important point to note is that when testing in the Airflow UI, the test executes from the webserver only. This feature is
Review Comment:
Hey @msumit! Thanks for taking a look. I'm a bit confused, is testing with the Airflow UI _not_ running the test from the webserver?
I changed the original sentence a little to try and make it clear the "webserver only" part was tied to testing with the Airflow UI.
> One important point to note is that when testing in the Airflow UI, the test executes from the webserver only.
Is this statement causing some confusion or needs to be more clear? Or is the word "only" make it sound not so clear?
##########
docs/apache-airflow/howto/connection.rst:
##########
@@ -191,19 +191,23 @@ Passwords cannot be manipulated or read without the key. For information on conf
Testing Connections
^^^^^^^^^^^^^^^^^^^
-Airflow Web UI & API allows to test connections. The test connection feature can be used from
-:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page, or through calling
-:doc:`Connections REST API </stable-rest-api-ref/>`.
+Airflow Web UI, REST API, and CLI allow you to test connections. The test connection feature can be used from
+:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page in the UI, through calling
+:doc:`Connections REST API </stable-rest-api-ref/>`, or running the ``airflow connections test`` :ref:`CLI command <cli>`.
-To test a connection Airflow calls out the ``test_connection`` method from the associated hook class and reports the
-results of it. It may happen that the connection type does not have any associated hook or the hook doesn't have the
-``test_connection`` method implementation, in either case the error message will throw the proper error message.
+To test a connection, Airflow calls the ``test_connection`` method from the associated hook class and reports the
+results. It may happen that the connection type does not have any associated hook or the hook doesn't have the
+``test_connection`` method implementation, in either case an error message will be displayed or functionality
+will be disabled (if you are testing in the UI).
-One important point to note is that the connections will be tested from the webserver only, so this feature is
+One important point to note is that when testing in the Airflow UI, the test executes from the webserver only. This feature is
Review Comment:
I can definitely add some explicit notes about different libs between webserver/workers (for the UI tests) and machines/pods (for CLI tests).
--
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] msumit commented on a diff in pull request #29892: Introduce testing a connection via Airflow CLI
Posted by "msumit (via GitHub)" <gi...@apache.org>.
msumit commented on code in PR #29892:
URL: https://github.com/apache/airflow/pull/29892#discussion_r1126775280
##########
docs/apache-airflow/howto/connection.rst:
##########
@@ -191,19 +191,23 @@ Passwords cannot be manipulated or read without the key. For information on conf
Testing Connections
^^^^^^^^^^^^^^^^^^^
-Airflow Web UI & API allows to test connections. The test connection feature can be used from
-:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page, or through calling
-:doc:`Connections REST API </stable-rest-api-ref/>`.
+Airflow Web UI, REST API, and CLI allow you to test connections. The test connection feature can be used from
+:ref:`create <creating_connection_ui>` or :ref:`edit <editing_connection_ui>` connection page in the UI, through calling
+:doc:`Connections REST API </stable-rest-api-ref/>`, or running the ``airflow connections test`` :ref:`CLI command <cli>`.
-To test a connection Airflow calls out the ``test_connection`` method from the associated hook class and reports the
-results of it. It may happen that the connection type does not have any associated hook or the hook doesn't have the
-``test_connection`` method implementation, in either case the error message will throw the proper error message.
+To test a connection, Airflow calls the ``test_connection`` method from the associated hook class and reports the
+results. It may happen that the connection type does not have any associated hook or the hook doesn't have the
+``test_connection`` method implementation, in either case an error message will be displayed or functionality
+will be disabled (if you are testing in the UI).
-One important point to note is that the connections will be tested from the webserver only, so this feature is
+One important point to note is that when testing in the Airflow UI, the test executes from the webserver only. This feature is
Review Comment:
I think the wording can be correct & we should remove the word `webserver only` and add about the machine/pod from where the CLI is being run.
--
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