From 78530846e15d005a35138b9f0e1d10333335f64c Mon Sep 17 00:00:00 2001 From: Mike McDonald Date: Fri, 14 Jan 2022 10:49:29 -0800 Subject: [PATCH 1/2] fix: Update custom connection field processing Fixes issue where custom connectionfields are not updated because `extra` field is in form and has previous values, overriding custom field values. Adds portion of connection form tests to test functionality. Ref: #20839 --- airflow/www/views.py | 28 ++++++++++++++---------- tests/www/views/test_views_connection.py | 15 +++++++++++++ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index a17f98eefb8f..adb2cea26368 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -3594,19 +3594,17 @@ 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 - if key in form.data and key.startswith(f"extra__{conn_type}__") - } - # If parameters are added to the classic `Extra` field, include these values along with - # custom-field extras. - extra_conn_params = form.data.get("extra") + # The extra value is the combination of custom fields for this conn_type and the original Extra field. + # The extra form field with all extra values (including custom fields) is in the form being processed + # so we store those values, and override them with anything in the custom fields mapped to specific extra keys + extra = {} + + extra_field = form.data.get("extra") - if extra_conn_params: + if extra_field: try: - extra.update(json.loads(extra_conn_params)) + extra.update(json.loads(extra_field)) except (JSONDecodeError, TypeError): flash( Markup( @@ -3615,11 +3613,19 @@ def process_form(self, form, is_created): "

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}", + f"{extra_field}", ), category="error", ) + custom_fields = { + key: form.data[key] + for key in self.extra_fields + if key in form.data and key.startswith(f"extra__{conn_type}__") + } + + extra.update(custom_fields) + if extra.keys(): form.extra.data = json.dumps(extra) diff --git a/tests/www/views/test_views_connection.py b/tests/www/views/test_views_connection.py index f26809cc8ebb..d0fb165be3f3 100644 --- a/tests/www/views/test_views_connection.py +++ b/tests/www/views/test_views_connection.py @@ -107,6 +107,21 @@ def test_process_form_extras(): assert json.loads(mock_form.extra.data) == {"extra__test3__custom_field": "custom_field_val3"} + # Testing parameters set in both extra and custom fields (cunnection updates). + mock_form = mock.Mock() + mock_form.data = { + "conn_type": "test4", + "conn_id": "extras_test4", + "extra": '{"extra__test4__custom_field": "custom_field_val3"}', + "extra__test4__custom_field": "custom_field_val4", + } + + cmv = ConnectionModelView() + cmv.extra_fields = ["extra__test4__custom_field"] # Custom field + cmv.process_form(form=mock_form, is_created=True) + + assert json.loads(mock_form.extra.data) == {"extra__test4__custom_field": "custom_field_val4"} + def test_duplicate_connection(admin_client): """Test Duplicate multiple connection with suffix""" From e0a18a8e961fd2cd32022b332c8af52ef9e0ca4c Mon Sep 17 00:00:00 2001 From: Mike McDonald Date: Fri, 21 Jan 2022 10:49:01 -0800 Subject: [PATCH 2/2] fix: Shorten comment line lengths --- airflow/www/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index adb2cea26368..dc9be2ddd0be 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -3595,9 +3595,9 @@ def process_form(self, form, is_created): conn_type = form.data['conn_type'] conn_id = form.data["conn_id"] - # The extra value is the combination of custom fields for this conn_type and the original Extra field. + # The extra value is the combination of custom fields for this conn_type and the Extra field. # The extra form field with all extra values (including custom fields) is in the form being processed - # so we store those values, and override them with anything in the custom fields mapped to specific extra keys + # so we start with those values, and override them with anything in the custom fields. extra = {} extra_field = form.data.get("extra")