[GitHub] ashb commented on a change in pull request #3684: [AIRFLOW-2840] - add update connections cli option

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

ashb commented on a change in pull request #3684: [AIRFLOW-2840] - add update connections cli option
URL: https://github.com/apache/airflow/pull/3684#discussion_r247172825
 
 

 ##########
 File path: airflow/bin/cli.py
 ##########
 @@ -1110,133 +1108,77 @@ def version(args):
 @cli_utils.action_logging
 def connections(args):
     if args.list:
-        # Check that no other flags were passed to the command
-        invalid_args = list()
-        for arg in ['conn_id', 'conn_uri', 'conn_extra'] + alternative_conn_specs:
-            if getattr(args, arg) is not None:
-                invalid_args.append(arg)
-        if invalid_args:
-            msg = ('\n\tThe following args are not compatible with the ' +
-                   '--list flag: {invalid!r}\n')
-            msg = msg.format(invalid=invalid_args)
-            print(msg)
-            return
-
-        with db.create_session() as session:
-            conns = session.query(Connection.conn_id, Connection.conn_type,
-                                  Connection.host, Connection.port,
-                                  Connection.is_encrypted,
-                                  Connection.is_extra_encrypted,
-                                  Connection.extra).all()
-            conns = [map(reprlib.repr, conn) for conn in conns]
-            msg = tabulate(conns, ['Conn Id', 'Conn Type', 'Host', 'Port',
-                                   'Is Encrypted', 'Is Extra Encrypted', 'Extra'],
-                           tablefmt="fancy_grid")
-            if sys.version_info[0] < 3:
-                msg = msg.encode('utf-8')
-            print(msg)
-            return
+        conns = api_client.list_connections()
+        # format it for the cli
+        conns = list(map(lambda c: (c['conn_id'], c['conn_type'],
+                                    c['host'], c['port'],
+                                    c['is_encrypted'],
+                                    c['is_extra_encrypted'],
+                                    c['extra']), conns))
+        conns = [map(reprlib.repr, conn) for conn in conns]
+        msg = tabulate(conns, ['Conn Id', 'Conn Type', 'Host', 'Port',
+                               'Is Encrypted', 'Is Extra Encrypted', 'Extra'],
+                       tablefmt="fancy_grid")
+        if sys.version_info[0] < 3:
+            msg = msg.encode('utf-8')
+        print(msg)
+        return
 
     if args.delete:
-        # Check that only the `conn_id` arg was passed to the command
-        invalid_args = list()
-        for arg in ['conn_uri', 'conn_extra'] + alternative_conn_specs:
-            if getattr(args, arg) is not None:
-                invalid_args.append(arg)
-        if invalid_args:
-            msg = ('\n\tThe following args are not compatible with the ' +
-                   '--delete flag: {invalid!r}\n')
-            msg = msg.format(invalid=invalid_args)
-            print(msg)
-            return
-
-        if args.conn_id is None:
-            print('\n\tTo delete a connection, you Must provide a value for ' +
-                  'the --conn_id flag.\n')
-            return
-
-        with db.create_session() as session:
-            try:
-                to_delete = (session
-                             .query(Connection)
-                             .filter(Connection.conn_id == args.conn_id)
-                             .one())
-            except exc.NoResultFound:
-                msg = '\n\tDid not find a connection with `conn_id`={conn_id}\n'
-                msg = msg.format(conn_id=args.conn_id)
-                print(msg)
-                return
-            except exc.MultipleResultsFound:
-                msg = ('\n\tFound more than one connection with ' +
-                       '`conn_id`={conn_id}\n')
-                msg = msg.format(conn_id=args.conn_id)
-                print(msg)
-                return
-            else:
-                deleted_conn_id = to_delete.conn_id
-                session.delete(to_delete)
-                msg = '\n\tSuccessfully deleted `conn_id`={conn_id}\n'
-                msg = msg.format(conn_id=deleted_conn_id)
-                print(msg)
-            return
+        try:
+            print(api_client.delete_connection(args.conn_id, args.delete_all))
+        except MissingArgument as ma:
+            print(ma)
+        return
 
     if args.add:
-        # Check that the conn_id and conn_uri args were passed to the command:
-        missing_args = list()
-        invalid_args = list()
-        if not args.conn_id:
-            missing_args.append('conn_id')
-        if args.conn_uri:
-            for arg in alternative_conn_specs:
-                if getattr(args, arg) is not None:
-                    invalid_args.append(arg)
-        elif not args.conn_type:
-            missing_args.append('conn_uri or conn_type')
-        if missing_args:
-            msg = ('\n\tThe following args are required to add a connection:' +
-                   ' {missing!r}\n'.format(missing=missing_args))
-            print(msg)
-        if invalid_args:
-            msg = ('\n\tThe following args are not compatible with the ' +
-                   '--add flag and --conn_uri flag: {invalid!r}\n')
-            msg = msg.format(invalid=invalid_args)
+        try:
+            new_conn = api_client.add_connection(
 
 Review comment:
   Might be easier to do `new_conn = api_client.add_connection(vars(args))`? (that does rely on the names from argparser matching the named args in the api client, but they do right now)
   
   Not sure on this one.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services