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/05/12 10:24:49 UTC

[GitHub] [airflow] msumit opened a new pull request #15795: [WIP] Ability to test connections from UI or API

msumit opened a new pull request #15795:
URL: https://github.com/apache/airflow/pull/15795


   WIP - Adding test connections support from UI or API. It'll store the test status & message into DB and will use that on UI to display the information later on. 
   
   <!--
   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/
   -->
   
   ---
   **^ 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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637915420



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +153,67 @@ $(document).ready(function () {
     }
   }
 
+  /**
+   * Produces JSON stringified data from a html form data
+   *
+   * @param {string} selector Jquery from selector string.
+   * @returns {string} Form data as a JSON string
+   */
+  function getSerializedFormData(selector) {
+    const outObj = {};
+    const inArray = $(selector).serializeArray();
+
+    $.each(inArray, function () {
+      if (this.name === 'conn_id') {
+        outObj.connection_id = this.value;
+      } else if (this.value !== '' && this.name !== 'csrf_token') {
+        outObj[this.name] = this.value;
+      }
+    });
+
+    return JSON.stringify(outObj);
+  }
+
+  /**
+   * Displays the Flask style alert on UI via JS
+   *
+   * @param {boolean} status - true for success, false for error
+   * @param {strinf} message - The text message to show in alert box

Review comment:
       ```suggestion
      * @param {string} message - The text message to show in alert box
   ```
   Just a typo




-- 
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] msumit commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636704895



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +133,21 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_CREATE, permissions.RESOURCE_CONNECTION)])
+def test_connection():
+    """Test a connection"""
+    body = request.json
+    try:
+        data = connection_schema.load(body)
+        dummy_conn_id = get_random_string()
+        data['conn_id'] = dummy_conn_id
+        conn = Connection(**data)
+        conn_env_var = f'AIRFLOW_CONN_{dummy_conn_id.upper()}'
+        os.environ[conn_env_var] = conn.get_uri()

Review comment:
       Yes, but some hooks tried to get the conn object from their __init__, and as we are not storing anything in DB, they fail. Setting it in env let them use the 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] kaxil commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636307278



##########
File path: docs/apache-airflow/howto/connection.rst
##########
@@ -360,6 +364,24 @@ In addition to retrieving connections from environment variables or the metastor
 an secrets backend to retrieve connections. For more details see :doc:`/security/secrets/secrets-backend/index`.
 
 
+Test 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/>`.
+
+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.

Review comment:
       ```suggestion
   ``test_connection`` method implementation, in either case the error message will throw the proper error message.
   ```




-- 
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] ephraimbuddy commented on a change in pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r633736653



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +130,24 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_EDIT, permissions.RESOURCE_CONNECTION)])
+@provide_session
+def test_connection(connection_id, session):
+    """Test a connection"""
+    try:
+        data = connection_schema.load(request.json, partial=True)

Review comment:
       I’ll suggest that we make this endpoint a GET since what we need is to run test_connection() on a connection instance




-- 
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] ashb commented on a change in pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r632508255



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -369,6 +369,27 @@ paths:
         '404':
           $ref: '#/components/responses/NotFound'
 
+  /connections/{connection_id}/test:

Review comment:
       As Kaxil mentioned, it would be nice to have a test button for the edit form, so we'd need ` /connections/test` POST end point that creates the connection and tests it rather than loading one from the DB.
   
   We possibly don't need this endpoint at all, as to me the use of an test button would be on the edit form, where we can just submit the full details payload.




-- 
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] ephraimbuddy commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r633736653



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +130,24 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_EDIT, permissions.RESOURCE_CONNECTION)])
+@provide_session
+def test_connection(connection_id, session):
+    """Test a connection"""
+    try:
+        data = connection_schema.load(request.json, partial=True)

Review comment:
       I’ll suggest that we make this endpoint a GET since what we need is to run test_connection() on a connection instance




-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839812421


   > Completely agree with you @potiuk here. In fact, I had similar questions about this feature. But then with my experience of working with a couple of companies over the past 5yrs, the webserver is running the same code of what workers are running, so the feature would be useful for such installations. For those, who don't have access from the webserver, it can act as a quick validator :).
   
   Yeah, agree it's an edge case. Note that it''s not only the "image" of webserver vs. worker  or "packages installed" - but also the "environment" it is running at. For example in our K8S deployment we had a side-car running next to the worker that kept refreshing kerberos tokens and this side-car shared a volume with the worker (and only worker had this enabled).
   
   But yeah I think in vast majority of cases it will work. Maybe a good solution to that will be to be very explicit about it - i.e. in a comment/tooltip/API description be very explicit that this is the status of connections from webserver and it might be different if run from the workers? 


-- 
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] msumit commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636705796



##########
File path: airflow/models/connection.py
##########
@@ -348,6 +348,25 @@ def debug_info(self):
             self.extra_dejson,
         )
 
+    def test_connection(self):
+        """Calls out get_hook method and executes test_connection method on that."""
+        status, message = False, ''
+        try:
+            hook = self.get_hook()
+            if hook and getattr(hook, 'test_connection', False):
+                status, message = hook.test_connection()
+            else:
+                message = (
+                    f"Hook {hook.__class__.__name__} doesn't implement or inherit test_connection method"

Review comment:
       yeah, in that the message would be `Hook NoneType doesn't...`




-- 
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] kaxil edited a comment on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839716702


   > Nice! One small (potential) issue - which might be worth thinking about here. I think often the Webserver is limited in connectivity. I know for a fact that MWAA and Composer have a much more restricted "environment" to run webserver on due to sensitivity of the webserver (it is exposed to outside world). I am not sure if they are limiting the outgoing connections from webserver, but I think they might @subashcanapathy ? or at least might want to limit it in the future.
   > 
   > Also there are are number of connections that might require the worker "environment" to work on (for example GOOGLE_APPLICATION_CREDENTIALS for gcp, or worker-identity configured for pod they are running on, or Kerberos configured for workers and only for workers (I've worked for a customer that had an environment where only the workers had the credentials that allowed them to make outgoing connections and authenticate using Kerberos).
   > 
   > This is not a blocker for that change, but just something to remember about - that it fhe "connection" test does not work via API/Webserver (the test will be executed in the webserver instance), it does not mean that the connection is not working from workers. It might be worth-while to add a "last successful connection from worker" - so basically store when last time the connection succeeded for actual worker running. This would give more complete information about the status of the connection.
   
   I consider this more of an "I have added a new connection now, I want to test if it is working or not" as a user can validate before they put or use the connection in their DAG. So not "when was this connection last successfully" as the connection to "test" like running a "select 1" query. We can extend this feature once this PR is merged to more connections and to "CLI".
   
   Actually "testing" will help us propagate the error message to UI, API (later CLI) unlike clubbing it to an exiting usage of connection is some Hook or Operator.
   
   Also, once we have all the Hooks with this method, users/companies can use it as a pre-validation task.


-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636309175



##########
File path: airflow/models/connection.py
##########
@@ -348,6 +348,25 @@ def debug_info(self):
             self.extra_dejson,
         )
 
+    def test_connection(self):
+        """Calls out get_hook method and executes test_connection method on that."""
+        status, message = False, ''
+        try:
+            hook = self.get_hook()
+            if hook and getattr(hook, 'test_connection', False):
+                status, message = hook.test_connection()
+            else:
+                message = (
+                    f"Hook {hook.__class__.__name__} doesn't implement or inherit test_connection method"

Review comment:
       This could also be no hook class found.

##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };

Review comment:
       I think this functionality is built in to jQuery.
   
   /cc @bbovenzi 




-- 
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] ashb commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-845920577


   I wonder if we should special case and detect the case when the API is disabled and disable the form control with a tool tip?


-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636862403



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +133,21 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_CREATE, permissions.RESOURCE_CONNECTION)])
+def test_connection():
+    """Test a connection"""
+    body = request.json
+    try:
+        data = connection_schema.load(body)
+        dummy_conn_id = get_random_string()
+        data['conn_id'] = dummy_conn_id
+        conn = Connection(**data)
+        conn_env_var = f'AIRFLOW_CONN_{dummy_conn_id.upper()}'

Review comment:
       ```suggestion
           from airflow.secrets.environment_variables import CONN_ENV_PREFIX
           conn_env_var = f'{CONN_ENV_PREFIX}{dummy_conn_id.upper()}'
   ```
   
   (feel free to move the import somewhere more appropriate.




-- 
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] msumit commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636875999



##########
File path: airflow/www/templates/airflow/conn_edit_form.html
##########
@@ -0,0 +1,81 @@
+{#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#}
+
+{% import 'appbuilder/general/lib.html' as lib %}

Review comment:
       Need to override this [line](https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/templates/appbuilder/general/widgets/form.html#L53) 




-- 
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] msumit commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637737138



##########
File path: airflow/models/connection.py
##########
@@ -348,6 +348,25 @@ def debug_info(self):
             self.extra_dejson,
         )
 
+    def test_connection(self):
+        """Calls out get_hook method and executes test_connection method on that."""
+        status, message = False, ''
+        try:
+            hook = self.get_hook()
+            if hook and getattr(hook, 'test_connection', False):
+                status, message = hook.test_connection()
+            else:
+                message = (
+                    f"Hook {hook.__class__.__name__} doesn't implement or inherit test_connection method"

Review comment:
       Ok, I was wrong, the `get_hook` does not return a `None`. It raises an exception if the hook is not found. 




-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637915667



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -68,6 +68,19 @@ $(document).ready(function () {
   const controlsContainer = getControlsContainer();
   const connTypesToControlsMap = getConnTypesToControlsMap();
 
+  // Create a test connection button & insert it right next to the save (submit) button
+  const testConnBtn = $('<button id="test-connection" type="button" class="btn btn-sm btn-primary" ' +
+    'style="margin-left: 3px; pointer-events: all">Test\n <i class="fa fa-rocket"></i></button>');
+
+  // eslint-disable-next-line no-undef

Review comment:
       When we do the `meta` tags we can remove this eslint rule




-- 
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] kaxil commented on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839716702


   > Nice! One small (potential) issue - which might be worth thinking about here. I think often the Webserver is limited in connectivity. I know for a fact that MWAA and Composer have a much more restricted "environment" to run webserver on due to sensitivity of the webserver (it is exposed to outside world). I am not sure if they are limiting the outgoing connections from webserver, but I think they might @subashcanapathy ? or at least might want to limit it in the future.
   > 
   > Also there are are number of connections that might require the worker "environment" to work on (for example GOOGLE_APPLICATION_CREDENTIALS for gcp, or worker-identity configured for pod they are running on, or Kerberos configured for workers and only for workers (I've worked for a customer that had an environment where only the workers had the credentials that allowed them to make outgoing connections and authenticate using Kerberos).
   > 
   > This is not a blocker for that change, but just something to remember about - that it fhe "connection" test does not work via API/Webserver (the test will be executed in the webserver instance), it does not mean that the connection is not working from workers. It might be worth-while to add a "last successful connection from worker" - so basically store when last time the connection succeeded for actual worker running. This would give more complete information about the status of the connection.
   
   I consider this more of an "I have added a new connection now, I want to test if it is working or not" as a user can validate before they put or use the connection in their DAG. So not "when was this connection last successfully" as the connection to "test" like running a "select 1" query. We can extend this feature once this PR is merged to more connections and to "CLI".
   
   Actually "testing" will help us propagate the error message to UI, API (later CLI) unlike clubbing it to an exiting usage of connection is some Hook or Operator.


-- 
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] msumit commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-845918626


   > Oh. Oh no.
   > 
   > By default, the REST API is set to deny_all, meaning that out of the box the test button won't work :(
   
   Yeah, just realized this while chatting with @bbovenzi.. we can handle the error in JS & let the user know the reason.. 


-- 
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] github-actions[bot] commented on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839707648


   [The Workflow run](https://github.com/apache/airflow/actions/runs/835109005) is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.


-- 
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] msumit commented on a change in pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r634499659



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -369,6 +369,27 @@ paths:
         '404':
           $ref: '#/components/responses/NotFound'
 
+  /connections/{connection_id}/test:

Review comment:
       I tried that approach as well, but we are relying on the `get_hook` method from the connection class and then calling `test_connection` from the hook. But in some hooks the init looks for a valid connection and errors out if not found ex. https://github.com/apache/airflow/blob/master/airflow/providers/databricks/hooks/databricks.py#L120




-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637915667



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -68,6 +68,19 @@ $(document).ready(function () {
   const controlsContainer = getControlsContainer();
   const connTypesToControlsMap = getConnTypesToControlsMap();
 
+  // Create a test connection button & insert it right next to the save (submit) button
+  const testConnBtn = $('<button id="test-connection" type="button" class="btn btn-sm btn-primary" ' +
+    'style="margin-left: 3px; pointer-events: all">Test\n <i class="fa fa-rocket"></i></button>');
+
+  // eslint-disable-next-line no-undef

Review comment:
       When we do the `meta` tags we can remove this eslint-disable rule




-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637931578



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -68,6 +72,19 @@ $(document).ready(function () {
   const controlsContainer = getControlsContainer();
   const connTypesToControlsMap = getConnTypesToControlsMap();
 
+  // Create a test connection button & insert it right next to the save (submit) button
+  const testConnBtn = $('<button id="test-connection" type="button" class="btn btn-sm btn-primary" ' +
+    'style="margin-left: 3px; pointer-events: all">Test\n <i class="fa fa-rocket"></i></button>');
+
+  // eslint-disable-next-line no-undef

Review comment:
       ```suggestion
   ```




-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636352746



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };

Review comment:
       Yes, it should be just `.serialize()` https://api.jquery.com/serialize/
   
   




-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636862851



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +133,21 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_CREATE, permissions.RESOURCE_CONNECTION)])
+def test_connection():
+    """Test a connection"""
+    body = request.json
+    try:
+        data = connection_schema.load(body)
+        dummy_conn_id = get_random_string()
+        data['conn_id'] = dummy_conn_id
+        conn = Connection(**data)
+        conn_env_var = f'AIRFLOW_CONN_{dummy_conn_id.upper()}'
+        os.environ[conn_env_var] = conn.get_uri()
+        status, message = conn.test_connection()
+        del os.environ[conn_env_var]

Review comment:
       In case of an exception this env var is not deleted -- we should delete this in a `finally` 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.

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

[GitHub] [airflow] msumit commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637751334



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };

Review comment:
       I would like to keep the APIs simple and not driven by the needs of UI. 




-- 
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] subashcanapathy commented on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
subashcanapathy commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839932430


   It comes down to adding a note that the test/feature is _subject to network egress rules setup for your webserver_. MWAA and Composer(likely?) will have plugin restrictions on the webserver fleet, and additionally the customer's VPC/PrivateNetwork might pose issues reaching to connection provider. 
   In the default setting, if the connections are stored in the providers that comes with the installation - this should work as is. If the customer is using a connections backend that requires a pip install of additional package and/or a plugin, then this feature won't work for the customers on the webui.


-- 
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] msumit commented on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839684109


   Completely agree with you @potiuk here. In fact, I had similar questions about this feature. But then with my experience of working with a couple of companies over the past 5yrs, the webserver is running the same code of what workers are running, so the feature would be useful for such installations. For those, who don't have access from the webserver, it can act as a quick validator :).
   
   Updating things from the workers is also a good idea & for that, we can create a decorator which automatically updates the status behind the scene. But again, what if there are 2 or more Celery workers & are running in different VPCs? Or K8S pods with different images, some with correct creds & some not? 


-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-840010689


   Sounds good :D


-- 
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] github-actions[bot] commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-845428765


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839665975


   Nice! One small (potential) issue - which might be worth thinking about here. I think often the Webserver is limited in connectivity. I know for a fact that MWAA and Composer have a much more restricted "environment" to run webserver on due to sensitivity of the webserver (it is exposed to outside world). I am not sure if they are limiting the outgoing connections from webserver, but I think they might @subashcanapathy  ? or at least might want to limit it. Also there are are number of connections that might require the worker "environment" to work on (for example GOOGLE_APPLICATION_CREDENTIALS for gcp, or worker-identity configured for pod they are running on, or Kerberos configured for workers and only for workers (I've worked for a customer that had an environment where only the workers had the credentials that allowed them to make outgoing connections).
   
   This is not a blocker for that change, but just something to remember about - that it fhe "connection" test does not work via API/Webserver (the test will be executed in the webserver instance), it does not mean that the connection is not working from workers. It might be worth-while to add a "last successful connection from worker" - so basically have a time when last time the connection succeeded for actual worker running. This would give more complete information about the status of the 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] msumit commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-845089887


   @ashb @kaxil @potiuk @ephraimbuddy this PR can be reviewed now. 


-- 
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] github-actions[bot] commented on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-844107269


   [The Workflow run](https://github.com/apache/airflow/actions/runs/856986173) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636308114



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +133,21 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_CREATE, permissions.RESOURCE_CONNECTION)])
+def test_connection():
+    """Test a connection"""
+    body = request.json
+    try:
+        data = connection_schema.load(body)
+        dummy_conn_id = get_random_string()
+        data['conn_id'] = dummy_conn_id
+        conn = Connection(**data)
+        conn_env_var = f'AIRFLOW_CONN_{dummy_conn_id.upper()}'
+        os.environ[conn_env_var] = conn.get_uri()

Review comment:
       Why do we need to do this? We've already got the connection object created...




-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636867154



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };

Review comment:
       Ah gotcha.
   
   Another option would be to convert the API endpoint to _also_ accept www-form-encoded data?




-- 
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] msumit commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-846949411


   > I wonder if we should special case and detect the case when the API is disabled and disable the form control with a tool tip?
   
   done


-- 
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] github-actions[bot] commented on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-844107658


   [The Workflow run](https://github.com/apache/airflow/actions/runs/856988768) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839812421


   > Completely agree with you @potiuk here. In fact, I had similar questions about this feature. But then with my experience of working with a couple of companies over the past 5yrs, the webserver is running the same code of what workers are running, so the feature would be useful for such installations. For those, who don't have access from the webserver, it can act as a quick validator :).
   
   Yeah, agree it's an edge case. Note that it''s not only the "image" of webserver vs. worker  or "packages installed" - but also the "environment" it is running at. For example in our K8S deployment we had a side-car running next to the worker that kept refreshing kerberos tokens and this side-car shared a volume with the worker (and only worker had this enabled).
   
   But yeah I think in vast majority of cases it will work. Maybe a good solution to that will be to be very explicit about it - i.e. in a comment/tooltip/API description be very explicit that this is the status of connections from webserver and it might be different if run from the workers? 
   
   I just really want to avoid questions "My connection is reported as not working in the UI, What should i do to make it works?". It's not obvious for the potential user that this **might** be caused by the worker/vs. webserver configuration.


-- 
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] github-actions[bot] commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-846980186


   [The Workflow run](https://github.com/apache/airflow/actions/runs/871192935) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


-- 
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] ashb commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-845916860


   Oh. Oh no.
   
   By default, the REST API is set to deny_all, meaning that out of the box the test button won't work :(


-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636870358



##########
File path: airflow/www/templates/airflow/conn_edit_form.html
##########
@@ -0,0 +1,81 @@
+{#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#}
+
+{% import 'appbuilder/general/lib.html' as lib %}

Review comment:
       Where did this template come from, cos it doesn't look anything like https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/templates/appbuilder/general/model/edit.html
   
   The reason I'm asking is I'm wondering if we can do it with template inheritance (where we just re-define one block and then call `{{ super() }}` to make this smaller.




-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636352746



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };

Review comment:
       Yes, it can be just `.serialize()` https://api.jquery.com/serialize/ aside from scrubbing the csrf_token
   
   




-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839812421


   > Completely agree with you @potiuk here. In fact, I had similar questions about this feature. But then with my experience of working with a couple of companies over the past 5yrs, the webserver is running the same code of what workers are running, so the feature would be useful for such installations. For those, who don't have access from the webserver, it can act as a quick validator :).
   
   Yeah, agree it's an edge case. Note that it''s not only the "image" of webserver vs. worker  or "packages installed" - but also the "environment" it is running at. For example in our K8S deployment we had a side-car running next to the worker that kept refreshing kerberos tokens and this side-car shared a volume with the worker (and only worker had this enabled).
   
   But yeah I think in vast majority of cases it will work. Maybe a good solution to that will be to be very explicit about it - i.e. in a comment/tooltip/API description be very explicit that this is the status of connections from webserver and it might be different if run from the workers? 
   
   I just really want to avoid questions "My connection is reported as not working in the UI? What should i do to make it works". It's not obvious for the potential user that this **might** be caused by the worker/vs. webserver configuration.


-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839665975


   Nice! One small (potential) issue - which might be worth thinking about here. I think often the Webserver is limited in connectivity. I know for a fact that MWAA and Composer have a much more restricted "environment" to run webserver on due to sensitivity of the webserver (it is exposed to outside world). I am not sure if they are limiting the outgoing connections from webserver, but I think they might @subashcanapathy  ? or at least might want to limit it in the future. 
   
   Also there are are number of connections that might require the worker "environment" to work on (for example GOOGLE_APPLICATION_CREDENTIALS for gcp, or worker-identity configured for pod they are running on, or Kerberos configured for workers and only for workers (I've worked for a customer that had an environment where only the workers had the credentials that allowed them to make outgoing connections and authenticate using Kerberos).
   
   This is not a blocker for that change, but just something to remember about - that it fhe "connection" test does not work via API/Webserver (the test will be executed in the webserver instance), it does not mean that the connection is not working from workers. It might be worth-while to add a "last successful connection from worker" - so basically have a time when last time the connection succeeded for actual worker running. This would give more complete information about the status of the 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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636862656



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +133,21 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_CREATE, permissions.RESOURCE_CONNECTION)])
+def test_connection():
+    """Test a connection"""
+    body = request.json
+    try:
+        data = connection_schema.load(body)
+        dummy_conn_id = get_random_string()
+        data['conn_id'] = dummy_conn_id
+        conn = Connection(**data)
+        conn_env_var = f'AIRFLOW_CONN_{dummy_conn_id.upper()}'
+        os.environ[conn_env_var] = conn.get_uri()

Review comment:
       Oh yes, gotcha. Could you add a comment saying why we need this 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.

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

[GitHub] [airflow] msumit merged pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit merged pull request #15795:
URL: https://github.com/apache/airflow/pull/15795


   


-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636863418



##########
File path: airflow/models/connection.py
##########
@@ -348,6 +348,25 @@ def debug_info(self):
             self.extra_dejson,
         )
 
+    def test_connection(self):
+        """Calls out get_hook method and executes test_connection method on that."""
+        status, message = False, ''
+        try:
+            hook = self.get_hook()
+            if hook and getattr(hook, 'test_connection', False):
+                status, message = hook.test_connection()
+            else:
+                message = (
+                    f"Hook {hook.__class__.__name__} doesn't implement or inherit test_connection method"

Review comment:
       Which is a bit confusing to a user if it happens -- I think we should have a different message for that case.




-- 
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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636879142



##########
File path: airflow/www/templates/airflow/conn_edit_form.html
##########
@@ -0,0 +1,81 @@
+{#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#}
+
+{% import 'appbuilder/general/lib.html' as lib %}

Review comment:
       Might be best to  create a Form python object and add the control that way then?




-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839665975


   Nice! One small (potential) issue - which might be worth thinking about here. I think often the Webserver is limited in connectivity. I know for a fact that MWAA and Composer have a much more restricted "environment" to run webserver on due to sensitivity of the webserver (it is exposed to outside world). I am not sure if they are limiting the outgoing connections from webserver, but I think they might @subashcanapathy  ? or at least might want to limit it in the future. 
   
   Also there are are number of connections that might require the worker "environment" to work on (for example GOOGLE_APPLICATION_CREDENTIALS for gcp, or worker-identity configured for pod they are running on, or Kerberos configured for workers and only for workers (I've worked for a customer that had an environment where only the workers had the credentials that allowed them to make outgoing connections).
   
   This is not a blocker for that change, but just something to remember about - that it fhe "connection" test does not work via API/Webserver (the test will be executed in the webserver instance), it does not mean that the connection is not working from workers. It might be worth-while to add a "last successful connection from worker" - so basically have a time when last time the connection succeeded for actual worker running. This would give more complete information about the status of the 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] ashb commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636868848



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };
+
+  function displayAlert(status, message){
+    const alertClass = status ? 'alert-success' : 'alert-error';
+    let alertBox = $('.container .row .alert');
+    if (alertBox.length) {
+      alertBox.removeClass('alert-success').removeClass('alert-error');
+      alertBox.addClass(alertClass);
+      alertBox.text(message);
+      alertBox.show();
+    } else {
+      alertBox = $('<div class="alert ' + alertClass + '">\n' +
+                   '<button type="button" class="close" data-dismiss="alert">×</button>\n' + message + '</div>');
+
+      $('.container .row').prepend(alertBox).show();
+    }
+  }
+
+  function testConnection() {
+    $.ajax({
+      url: '/api/v1/connections/test',
+      type: 'post',
+      contentType: 'application/json',
+      dataType: 'json',
+      data: JSON.stringify($('form#model_form').serializeObject()),
+      success: function(data) {
+        displayAlert(data.status, data.message);
+      },

Review comment:
       We should have an error handler here too




-- 
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] msumit commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636707667



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };

Review comment:
       @bbovenzi yeah, I tried `serialize` but it was giving me a string like `key1=val1&key2=val2` which again I need to convert into a proper json object. 




-- 
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] ephraimbuddy commented on a change in pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r633736653



##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -129,3 +130,24 @@ def post_connection(session):
         session.commit()
         return connection_schema.dump(connection)
     raise AlreadyExists(detail=f"Connection already exist. ID: {conn_id}")
+
+
+@security.requires_access([(permissions.ACTION_CAN_EDIT, permissions.RESOURCE_CONNECTION)])
+@provide_session
+def test_connection(connection_id, session):
+    """Test a connection"""
+    try:
+        data = connection_schema.load(request.json, partial=True)

Review comment:
       I’ll suggest that we make this endpoint a GET since what we need is to run test_connection() on a connection instance




-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839812421


   > Completely agree with you @potiuk here. In fact, I had similar questions about this feature. But then with my experience of working with a couple of companies over the past 5yrs, the webserver is running the same code of what workers are running, so the feature would be useful for such installations. For those, who don't have access from the webserver, it can act as a quick validator :).
   
   Yeah, agree it's an edge case. Not that it''s not only the "image" of webserver vs. worker  or "packages installed" - but also the "environment" it is running at. For example in our K8S deployment we had a side-car running next to the worker that kept refreshing kerberos tokens and this side-car shared a volume with the worker (and only worker had this enabled).
   
   But yeah I think in vast majority of cases it will work. Maybe a good solution to that will be to be very explicit about it - i.e. in a comment/tooltip/API description be very explicit that this is the status of connections from webserver and it might be different if run from the workers? 


-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636353045



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };
+
+  function displayAlert(status, message){
+    const alertClass = status ? 'alert-success' : 'alert-error';
+    let alertBox = $('.container .row .alert');
+    if (alertBox.length) {
+      alertBox.removeClass('alert-success').removeClass('alert-error');
+      alertBox.addClass(alertClass);
+      alertBox.text(message);
+      alertBox.show();
+    } else {
+      alertBox = $('<div class="alert ' + alertClass + '">\n' +
+                   '<button type="button" class="close" data-dismiss="alert">×</button>\n' + message + '</div>');
+
+      $('.container .row').prepend(alertBox).show();
+    }
+  }
+
+  function testConnection() {
+    $.ajax({
+      url: '/api/v1/connections/test',
+      type: 'post',
+      contentType: 'application/json',
+      dataType: 'json',
+      data: JSON.stringify($('form#model_form').serializeObject()),

Review comment:
       Do we need to both serialize and stringify the form data?




-- 
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] github-actions[bot] commented on pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-847006892


   [The Workflow run](https://github.com/apache/airflow/actions/runs/871303773) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637101562



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };
+
+  function displayAlert(status, message){
+    const alertClass = status ? 'alert-success' : 'alert-error';
+    let alertBox = $('.container .row .alert');
+    if (alertBox.length) {
+      alertBox.removeClass('alert-success').removeClass('alert-error');
+      alertBox.addClass(alertClass);
+      alertBox.text(message);
+      alertBox.show();
+    } else {
+      alertBox = $('<div class="alert ' + alertClass + '">\n' +
+                   '<button type="button" class="close" data-dismiss="alert">×</button>\n' + message + '</div>');
+
+      $('.container .row').prepend(alertBox).show();
+    }
+  }
+
+  function testConnection() {
+    $.ajax({
+      url: '/api/v1/connections/test',
+      type: 'post',
+      contentType: 'application/json',
+      dataType: 'json',
+      data: JSON.stringify($('form#model_form').serializeObject()),
+      success: function(data) {
+        displayAlert(data.status, data.message);
+      },
+    });
+  }
+
+  $('#test-connection').on('click', (e) => {
+    e.preventDefault();
+    testConnection();

Review comment:
       I think we're nesting the testConnection functions too much. `click -> testConnection() -> .serializeObject()`. Since each function is just used once lets just have all of it live inside of here for code readability.

##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {

Review comment:
       ```suggestion
         } else if (this.value !== '' && this.name !== 'csrf_token') {
   ```
   
   We shouldn't have an if statement that does nothing.

##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {

Review comment:
       We shouldn't add a prototype to jquery. Let's just move all of it to the `on('click'` handler.

##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();

Review comment:
       We need better variable names here. Single letters are only for common items event and id.

##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };
+
+  function displayAlert(status, message){
+    const alertClass = status ? 'alert-success' : 'alert-error';
+    let alertBox = $('.container .row .alert');
+    if (alertBox.length) {
+      alertBox.removeClass('alert-success').removeClass('alert-error');
+      alertBox.addClass(alertClass);
+      alertBox.text(message);
+      alertBox.show();
+    } else {
+      alertBox = $('<div class="alert ' + alertClass + '">\n' +
+                   '<button type="button" class="close" data-dismiss="alert">×</button>\n' + message + '</div>');
+
+      $('.container .row').prepend(alertBox).show();
+    }
+  }
+
+  function testConnection() {
+    $.ajax({
+      url: '/api/v1/connections/test',
+      type: 'post',
+      contentType: 'application/json',
+      dataType: 'json',
+      data: JSON.stringify($('form#model_form').serializeObject()),

Review comment:
       We should process the data all at once and not inline of the ajax call.




-- 
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 #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839665975


   Nice! One small (potential) issue - which might be worth thinking about here. I think often the Webserver is limited in connectivity. I know for a fact that MWAA and Composer have a much more restricted "environment" to run webserver on due to sensitivity of the webserver (it is exposed to outside world). I am not sure if they are limiting the outgoing connections from webserver, but I think they might @subashcanapathy  ? or at least might want to limit it in the future. 
   
   Also there are are number of connections that might require the worker "environment" to work on (for example GOOGLE_APPLICATION_CREDENTIALS for gcp, or worker-identity configured for pod they are running on, or Kerberos configured for workers and only for workers (I've worked for a customer that had an environment where only the workers had the credentials that allowed them to make outgoing connections and authenticate using Kerberos).
   
   This is not a blocker for that change, but just something to remember about - that it fhe "connection" test does not work via API/Webserver (the test will be executed in the webserver instance), it does not mean that the connection is not working from workers. It might be worth-while to add a "last successful connection from worker" - so basically store when last time the connection succeeded for actual worker running. This would give more complete information about the status of the 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] msumit commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636708108



##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
     }
   }
 
+  $.fn.serializeObject = function () {
+    const o = {};
+    const a = this.serializeArray();
+    $.each(a, function () {
+      if (this.name === 'csrf_token') {
+        // dont send csrf token
+      } else if (this.name === 'conn_id') {
+        o.connection_id = this.value;
+      } else if (this.value !== '') {
+        o[this.name] = this.value;
+      }
+    });
+    return o;
+  };
+
+  function displayAlert(status, message){
+    const alertClass = status ? 'alert-success' : 'alert-error';
+    let alertBox = $('.container .row .alert');
+    if (alertBox.length) {
+      alertBox.removeClass('alert-success').removeClass('alert-error');
+      alertBox.addClass(alertClass);
+      alertBox.text(message);
+      alertBox.show();
+    } else {
+      alertBox = $('<div class="alert ' + alertClass + '">\n' +
+                   '<button type="button" class="close" data-dismiss="alert">×</button>\n' + message + '</div>');
+
+      $('.container .row').prepend(alertBox).show();
+    }
+  }
+
+  function testConnection() {
+    $.ajax({
+      url: '/api/v1/connections/test',
+      type: 'post',
+      contentType: 'application/json',
+      dataType: 'json',
+      data: JSON.stringify($('form#model_form').serializeObject()),

Review comment:
       @bbovenzi I think yes, to match the input expected by the rest api. 




-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637914617



##########
File path: airflow/www/templates/airflow/conn_create.html
##########
@@ -21,6 +21,10 @@
 
 {% block tail %}
   {{ super() }}
+  <script>
+    // eslint-disable-next-line no-unused-vars
+    const restApiEnabled = '{{ rest_api_enabled }}' === 'True';

Review comment:
       See how we use `import getMetaValue from './meta_value';` in other js files. We should do the same here.




-- 
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] bbovenzi commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637914967



##########
File path: airflow/www/templates/airflow/conn_edit.html
##########
@@ -21,6 +21,10 @@
 
 {% block tail %}
   {{ super() }}
+  <script>
+    // eslint-disable-next-line no-unused-vars
+    const restApiEnabled = '{{ rest_api_enabled }}' === 'True';

Review comment:
       Let's use `<Meta>` here too.




-- 
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] kaxil commented on a change in pull request #15795: Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r636307158



##########
File path: docs/apache-airflow/howto/connection.rst
##########
@@ -360,6 +364,24 @@ In addition to retrieving connections from environment variables or the metastor
 an secrets backend to retrieve connections. For more details see :doc:`/security/secrets/secrets-backend/index`.
 
 
+Test 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/>`.
+
+To test a connection Airflow calls out the test_connection method from the associated hook class and reports the

Review comment:
       ```suggestion
   To test a connection Airflow calls out the ``test_connection`` method from the associated hook class and reports the
   ```




-- 
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] kaxil commented on pull request #15795: [WIP] Ability to test connections from UI or API

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#issuecomment-839984795


   > It comes down to adding a note that the test/feature is _subject to network egress rules setup for your webserver_. MWAA and Composer(likely?) will have plugin restrictions on the webserver fleet, and additionally the customer's VPC/PrivateNetwork might pose issues reaching to connection provider.
   > In the default setting, if the connections are stored in the providers that comes with the installation - this should work as is. If the customer is using a connections backend that requires a pip install of additional package and/or a plugin, then this feature won't work for the customers on the webui.
   
   Yup, since the API also only show connections from Connections saved in DB not the ones set via other secrets backend like Env Vars or Vault


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