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 2022/10/13 18:17:10 UTC

[GitHub] [airflow] dstandish opened a new pull request, #27041: Allow non-prefixed extras in AzureFileShareHook

dstandish opened a new pull request, #27041:
URL: https://github.com/apache/airflow/pull/27041

   From 2.3, non-prefixed extras are fully supported so we make this the preferred way.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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


[GitHub] [airflow] ferruzzi commented on a diff in pull request #27041: Allow non-prefixed extras in AzureFileShareHook

Posted by GitBox <gi...@apache.org>.
ferruzzi commented on code in PR #27041:
URL: https://github.com/apache/airflow/pull/27041#discussion_r1001057971


##########
airflow/providers/microsoft/azure/hooks/fileshare.py:
##########
@@ -76,41 +104,42 @@ def get_ui_field_behaviour() -> dict[str, Any]:
             "placeholders": {
                 'login': 'account name',
                 'password': 'secret',
-                'extra__azure_fileshare__sas_token': 'account url or token (optional)',
-                'extra__azure_fileshare__connection_string': 'account url or token (optional)',
-                'extra__azure_fileshare__protocol': 'account url or token (optional)',
+                'sas_token': 'account url or token (optional)',
+                'connection_string': 'account url or token (optional)',
+                'protocol': 'account url or token (optional)',
             },
         }
 
     def get_conn(self) -> FileService:
         """Return the FileService object."""
-        prefix = "extra__azure_fileshare__"
+
+        def check_for_conflict(key):
+            backcompat_key = f"{backcompat_prefix}{key}"
+            if backcompat_key in extras:
+                warnings.warn(
+                    f"Conflicting params `{key}` and `{backcompat_key}` found in extras for conn "
+                    f"{self.conn_id}. Using value for `{key}`.  Please ensure this is the correct value "
+                    f"and remove the backcompat key `{backcompat_key}`."
+                )
+
+        backcompat_prefix = "extra__azure_fileshare__"
         if self._conn:
             return self._conn
         conn = self.get_connection(self.conn_id)
-        service_options_with_prefix = conn.extra_dejson
+        extras = conn.extra_dejson
         service_options = {}
-        for key, value in service_options_with_prefix.items():
-            # in case dedicated FileShareHook is used, the connection will use the extras from UI.
-            # in case deprecated wasb hook is used, the old extras will work as well
-            if key.startswith(prefix):
-                if value != '':
-                    service_options[key[len(prefix) :]] = value
-                else:
-                    # warn if the deprecated wasb_connection is used
-                    warnings.warn(
-                        "You are using deprecated connection for AzureFileShareHook."
-                        " Please change it to `Azure FileShare`.",
-                        DeprecationWarning,
-                    )
-            else:
+        for key, value in extras.items():
+            if value == "":
+                continue
+            if not key.startswith('extra__'):
                 service_options[key] = value
-                # warn if the old non-prefixed value is used
-                warnings.warn(
-                    "You are using deprecated connection for AzureFileShareHook."
-                    " Please change it to `Azure FileShare`.",
-                    DeprecationWarning,
-                )
+                check_for_conflict(key)
+            elif key.startswith(backcompat_prefix):
+                eff_key = key[len(backcompat_prefix) :]

Review Comment:
   Not sure what eff_key is here, maybe that's a known term I'm just not familiar with, but it looks like it's the short (non-prefixed) key name, so maybe just call it that?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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


[GitHub] [airflow] dstandish merged pull request #27041: Allow and prefer non-prefixed extra fields for AzureFileShareHook

Posted by GitBox <gi...@apache.org>.
dstandish merged PR #27041:
URL: https://github.com/apache/airflow/pull/27041


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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