From 700e66aaa0af3d94fe235ccd6fce35c2a3904805 Mon Sep 17 00:00:00 2001 From: Josh Fell Date: Mon, 26 Jul 2021 23:08:55 -0400 Subject: [PATCH 1/4] Append classic Extra values with custom values --- airflow/www/views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/airflow/www/views.py b/airflow/www/views.py index 117b173f84fc3..2234bc9ee50a7 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -3226,6 +3226,11 @@ def process_form(self, form, is_created): for key in self.extra_fields if key in form.data and key.startswith(f"extra__{conn_type}__") } + + # If values are added to the classic `Extra` field, include these values along with custom-field extras. + if form.data.get("extra"): + extra.update(json.loads(form.data.get("extra"))) + if extra.keys(): form.extra.data = json.dumps(extra) From 0c192da6898b060597dab29b4e62de98b6781ebd Mon Sep 17 00:00:00 2001 From: Josh Fell Date: Mon, 26 Jul 2021 23:47:36 -0400 Subject: [PATCH 2/4] Small tweak to comment --- airflow/www/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index 2234bc9ee50a7..d8cfe645bda23 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -3227,7 +3227,8 @@ def process_form(self, form, is_created): if key in form.data and key.startswith(f"extra__{conn_type}__") } - # If values are added to the classic `Extra` field, include these values along with custom-field extras. + # If parameters are added to the classic `Extra` field, include these values along with + # custom-field extras. if form.data.get("extra"): extra.update(json.loads(form.data.get("extra"))) From 064a1b5ecc40dc1873914fd8e4f3b229c4def02e Mon Sep 17 00:00:00 2001 From: Josh Fell Date: Tue, 27 Jul 2021 12:42:21 -0400 Subject: [PATCH 3/4] Adding unit test for conn extras --- tests/www/views/test_views_connection.py | 52 ++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/www/views/test_views_connection.py b/tests/www/views/test_views_connection.py index 557697729a1be..249bf2a28e8f8 100644 --- a/tests/www/views/test_views_connection.py +++ b/tests/www/views/test_views_connection.py @@ -15,6 +15,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import json from unittest import mock import pytest @@ -56,6 +57,57 @@ def test_prefill_form_null_extra(): cmv.prefill_form(form=mock_form, pk=1) +def test_process_form_extras(): + """ + Test the handling of connection parameters set with the classic `Extra` field as well as custom fields. + """ + + # Testing parameters set in both `Extra` and custom fields. + mock_form = mock.Mock() + mock_form.data = { + "conn_type": "test", + "conn_id": "extras_test", + "extra": '{"param1": "param1_val"}', + "extra__test__custom_field": "custom_field_val", + } + + cmv = ConnectionModelView() + cmv.extra_fields = ["extra__test__custom_field"] # Custom field + cmv.process_form(form=mock_form, is_created=True) + + assert json.loads(mock_form.extra.data) == { + "extra__test__custom_field": "custom_field_val", + "param1": "param1_val", + } + + # Testing parameters set in `Extra` field only. + mock_form = mock.Mock() + mock_form.data = { + "conn_type": "test2", + "conn_id": "extras_test2", + "extra": '{"param2": "param2_val"}', + } + + cmv = ConnectionModelView() + cmv.process_form(form=mock_form, is_created=True) + + assert json.loads(mock_form.extra.data) == {"param2": "param2_val"} + + # Testing parameters set in custom fields only. + mock_form = mock.Mock() + mock_form.data = { + "conn_type": "test3", + "conn_id": "extras_test3", + "extra__test3__custom_field": "custom_field_val3", + } + + cmv = ConnectionModelView() + cmv.extra_fields = ["extra__test3__custom_field"] # Custom field + cmv.process_form(form=mock_form, is_created=True) + + assert json.loads(mock_form.extra.data) == {"extra__test3__custom_field": "custom_field_val3"} + + def test_duplicate_connection(admin_client): """Test Duplicate multiple connection with suffix""" conn1 = Connection( From 418cdbbc38497f5acc9b594cef9c1bb55ed10e5f Mon Sep 17 00:00:00 2001 From: Josh Fell Date: Thu, 29 Jul 2021 11:35:59 -0400 Subject: [PATCH 4/4] Adding flash error message --- airflow/www/views.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index d8cfe645bda23..f286cd9e3f133 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -3221,6 +3221,7 @@ def action_mulduplicate(self, connections, session=None): def process_form(self, form, is_created): """Process form data.""" conn_type = form.data['conn_type'] + conn_id = form.data["conn_id"] extra = { key: form.data[key] for key in self.extra_fields @@ -3229,8 +3230,23 @@ def process_form(self, form, is_created): # If parameters are added to the classic `Extra` field, include these values along with # custom-field extras. - if form.data.get("extra"): - extra.update(json.loads(form.data.get("extra"))) + extra_conn_params = form.data.get("extra") + + if extra_conn_params: + try: + extra.update(json.loads(extra_conn_params)) + except (JSONDecodeError, TypeError): + flash( + Markup( + "

The Extra connection field contained an invalid value for Conn ID: " + f"{conn_id}.

" + "

If connection parameters need to be added to Extra, " + "please make sure they are in the form of a single, valid JSON object.


" + "The following Extra parameters were not added to the connection:
" + f"{extra_conn_params}", + ), + category="error", + ) if extra.keys(): form.extra.data = json.dumps(extra)