[GitHub] [airflow] andres-lowrie commented on a change in pull request #9008: Get connections uri with AWS Secrets Manager backend

[GitBox <gi...@apache.org>]

andres-lowrie commented on a change in pull request #9008:
URL: https://github.com/apache/airflow/pull/9008#discussion_r539816366



##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -83,43 +90,121 @@ def client(self):
         )
         return session.client(service_name="secretsmanager", **self.kwargs)
 
-    def get_conn_uri(self, conn_id: str) -> Optional[str]:
+    def _get_user_and_password(self, secret):
+        for user_denomination in ['user', 'username', 'login']:
+            if user_denomination in secret:
+                user = secret[user_denomination]
+
+        for password_denomination in ['pass', 'password', 'key']:
+            if password_denomination in secret:
+                password = secret[password_denomination]
+
+        return user, password
+
+    def _get_host(self, secret):
+        for host_denomination in ['host', 'remote_host', 'server']:
+            if host_denomination in secret:
+                host = secret[host_denomination]
+
+        return host
+
+    def _get_conn_type(self, secret):
+        for type_word in ['conn_type', 'conn_id', 'connection_type', 'engine']:
+            if type_word in secret:
+                conn_type = secret[type_word]
+                conn_type = 'postgresql' if conn_type == 'redshift' else conn_type
+            else:
+                conn_type = 'connection'
+        return conn_type
+
+    def _get_port_and_database(self, secret):
+        if 'port' in secret:
+            port = secret['port']
+        else:
+            port = 5432
+        if 'database' in secret:
+            database = secret['database']
+        else:
+            database = ''
+
+        return port, database
+
+    def _get_extra(self, secret, conn_string):
+        if 'extra' in secret:
+            extra_dict = ast.literal_eval(secret['extra'])
+            counter = 0
+            for key, value in extra_dict.values():
+                if counter == 0:
+                    conn_string += f'{key}={value}'
+                else:
+                    conn_string += f'&{key}={value}'

Review comment:
       hello,
   
   I'm wondering if this approach would work for nested extra values.
   
   For example the `emr_default` one for aws looks like this: 
   https://github.com/apache/airflow/blob/c704293f75475671f11e240094fee36736adb1d9/airflow/utils/db.py#L205
   
   I think you mentioned something about using key tokens like `dont_use_*` or something like that  in order to have a way to ignore stuff; I'm just making a comment to make it visible




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