Skip to content

Commit

Permalink
Fix Slack Connections created in the UI (#26845)
Browse files Browse the repository at this point in the history
  • Loading branch information
Taragolis committed Oct 3, 2022
1 parent 2921796 commit 7b18307
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 1 deletion.
2 changes: 2 additions & 0 deletions airflow/providers/slack/hooks/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,13 @@ def get_connection_form_widgets(cls) -> dict[str, Any]:
from flask_appbuilder.fieldwidgets import BS3TextFieldWidget
from flask_babel import lazy_gettext
from wtforms import IntegerField, StringField
from wtforms.validators import NumberRange, Optional

return {
prefixed_extra_field("timeout", cls.conn_type): IntegerField(
lazy_gettext("Timeout"),
widget=BS3TextFieldWidget(),
validators=[Optional(strip_whitespace=True), NumberRange(min=1)],
description="Optional. The maximum number of seconds the client will wait to connect "
"and receive a response from Slack API.",
),
Expand Down
2 changes: 2 additions & 0 deletions airflow/providers/slack/hooks/slack_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,13 @@ def get_connection_form_widgets(cls) -> dict[str, Any]:
from flask_appbuilder.fieldwidgets import BS3TextFieldWidget
from flask_babel import lazy_gettext
from wtforms import IntegerField, StringField
from wtforms.validators import NumberRange, Optional

return {
prefixed_extra_field("timeout", cls.conn_type): IntegerField(
lazy_gettext("Timeout"),
widget=BS3TextFieldWidget(),
validators=[Optional(), NumberRange(min=1)],
description="Optional. The maximum number of seconds the client will wait to connect "
"and receive a response from Slack Incoming Webhook.",
),
Expand Down
7 changes: 6 additions & 1 deletion airflow/providers/slack/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ def get(self, field, default: Any = NOTSET):
:param default: If specified then use as default value if field not present in Connection Extra.
"""
prefixed_field = prefixed_extra_field(field, self.conn_type)
if prefixed_field in self.extra:
if prefixed_field in self.extra and self.extra[prefixed_field] not in (None, ""):
# Addition validation with non-empty required for connection which created in the UI
# in Airflow 2.2. In these connections always present key-value pair for all prefixed extras
# even if user do not fill this fields.
# In additional fields from `wtforms.IntegerField` might contain None value.
# E.g.: `{'extra__slackwebhook__proxy': '', 'extra__slackwebhook__timeout': None}`
return self.extra[prefixed_field]
elif field in self.extra:
return self.extra[field]
Expand Down
23 changes: 23 additions & 0 deletions tests/providers/slack/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,29 @@ def test_both_prefixed_and_not_in_extra_field(self, conn_type):
)
assert extra_config.get("arg1") == "bar"

@pytest.mark.parametrize("conn_type", ["slack", "slack_incoming_webhook"])
@pytest.mark.parametrize("empty_value", [None, ""])
def test_prefixed_extra_created_in_ui_connections(self, conn_type, empty_value):
"""Test that empty strings or None values in UI ignored."""
extra_config = ConnectionExtraConfig(
conn_type=conn_type,
conn_id="test-conn-id",
extra={
f"extra__{conn_type}__arg_missing": empty_value,
"arg_extra": "bar",
f"extra__{conn_type}__arg_extra": empty_value,
},
)
error_message = (
r"Couldn't find '.*' or '.*' in Connection \('.*'\) Extra and no default value specified\."
)
with pytest.raises(KeyError, match=error_message):
# No fallback should raise an error
extra_config.get("arg_missing")

assert extra_config.get("arg_missing", default="foo") == "foo"
assert extra_config.get("arg_extra") == "bar"

def test_get_parse_int(self):
extra_config = ConnectionExtraConfig(
conn_type="slack",
Expand Down

0 comments on commit 7b18307

Please sign in to comment.