[GitHub] [airflow] jmcarp commented on a change in pull request #4821: [AIRFLOW-3998] Use nested commands in cli.

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

jmcarp commented on a change in pull request #4821: [AIRFLOW-3998] Use nested commands in cli.
URL: https://github.com/apache/airflow/pull/4821#discussion_r275174229
 
 

 ##########
 File path: airflow/bin/cli.py
 ##########
 @@ -328,34 +348,44 @@ def pool_export_helper(filepath):
 
 
 @cli_utils.action_logging
-def variables(args):
-    if args.get:
-        try:
-            var = Variable.get(args.get,
-                               deserialize_json=args.json,
-                               default_var=args.default)
-            print(var)
-        except ValueError as e:
-            print(e)
-    if args.delete:
-        Variable.delete(args.delete)
-    if args.set:
-        Variable.set(args.set[0], args.set[1])
-    # Work around 'import' as a reserved keyword
-    imp = getattr(args, 'import')
-    if imp:
-        if os.path.exists(imp):
-            import_helper(imp)
-        else:
-            print("Missing variables file.")
-    if args.export:
-        export_helper(args.export)
-    if not (args.set or args.get or imp or args.export or args.delete):
-        # list all variables
-        with db.create_session() as session:
-            vars = session.query(Variable)
-            msg = "\n".join(var.key for var in vars)
-            print(msg)
+def variables_list(args):
+    with db.create_session() as session:
+        vars = session.query(Variable)
+    print("\n".join(var.key for var in vars))
+
+
+@cli_utils.action_logging
+def variables_get(args):
+    try:
+        var = Variable.get(args.key,
+                           deserialize_json=args.json,
+                           default_var=args.default)
+        print(var)
+    except ValueError as e:
+        print(e)
+
+
+@cli_utils.action_logging
+def variables_set(args):
+    Variable.set(args.key, args.value, serialize_json=args.json)
+
+
+@cli_utils.action_logging
+def variables_delete(args):
+    Variable.delete(args.key)
+
+
+@cli_utils.action_logging
+def variables_import(args):
+    if os.path.exists(args.file):
+        import_helper(args.file)
+    else:
+        print("Missing variables file.")
 
 Review comment:
   So far I haven't been changing the behavior of these commands, just changing the structure. Some error cases print a message and return without raising an error, and I agree that we should change that behavior. Any preference on whether that change happens as part of this patch or in a separate patch?

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


With regards,
Apache Git Services