[GitHub] [airflow] potiuk commented on a diff in pull request #28259: Speed up most Users/Role CLI commands

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

potiuk commented on code in PR #28259:
URL: https://github.com/apache/airflow/pull/28259#discussion_r1044495547


##########
airflow/cli/commands/user_command.py:
##########
@@ -49,42 +48,47 @@ class UserSchema(Schema):
 @suppress_logs_and_warning
 def users_list(args):
     """Lists users at the command line."""
-    appbuilder = cached_app().appbuilder
-    users = appbuilder.sm.get_all_users()
-    fields = ["id", "username", "email", "first_name", "last_name", "roles"]
+    from airflow.utils.cli_app_builder import get_application_builder
 
-    AirflowConsole().print_as(
-        data=users, output=args.output, mapper=lambda x: {f: x.__getattribute__(f) for f in fields}
-    )
+    with get_application_builder() as appbuilder:
+        users = appbuilder.sm.get_all_users()
+        fields = ["id", "username", "email", "first_name", "last_name", "roles"]
+
+        AirflowConsole().print_as(
+            data=users, output=args.output, mapper=lambda x: {f: x.__getattribute__(f) for f in fields}
+        )
 
 
 @cli_utils.action_cli(check_db=True)
 def users_create(args):
     """Creates new user in the DB."""
-    appbuilder = cached_app().appbuilder
-    role = appbuilder.sm.find_role(args.role)
-    if not role:
-        valid_roles = appbuilder.sm.get_all_roles()
-        raise SystemExit(f"{args.role} is not a valid role. Valid roles are: {valid_roles}")
-
-    if args.use_random_password:
-        password = "".join(random.choice(string.printable) for _ in range(16))
-    elif args.password:
-        password = args.password
-    else:
-        password = getpass.getpass("Password:")
-        password_confirmation = getpass.getpass("Repeat for confirmation:")
-        if password != password_confirmation:
-            raise SystemExit("Passwords did not match")
-
-    if appbuilder.sm.find_user(args.username):
-        print(f"{args.username} already exist in the db")
-        return
-    user = appbuilder.sm.add_user(args.username, args.firstname, args.lastname, args.email, role, password)
-    if user:
-        print(f'User "{args.username}" created with role "{args.role}"')
-    else:
-        raise SystemExit("Failed to create user")
+    from airflow.utils.cli_app_builder import get_application_builder
+
+    with get_application_builder() as appbuilder:

Review Comment:
   I do not want to make any more changes. Pareto's rule again - 20% of cost getting 80% gain.
   
   Sooner or later we WILL get rid of Fab in t's current form so those commands should be refactored then. For now - I think we have to live what we have and make our dependencies smaller and smaller to the point that such a replacement will be easy.



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