Rework kci user with new framework
#2141
Conversation
gctucker
force-pushed
the
kci-user-rework
branch
from
178e744
to
c0eb3e5
Compare
gctucker
force-pushed
the
kci-user-rework
branch
3 times, most recently
from
f7a51b0
to
987b6e1
Compare
gctucker
force-pushed
the
kci-user-rework
branch
4 times, most recently
from
9d37726
to
5f2e150
Compare
|
@gctucker Is this ready for review? |
gctucker
force-pushed
the
kci-user-rework
branch
from
5f2e150
to
8c6736b
Compare
It is now I've rebased it ;) |
Add a first implementation of 'kci user' with just the whoami command moved from the top-level kci script. Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
Implement the main parts of the `kci user` subcommand with whoami, find, token, password update / add. Some missing features on the API side mean we can't yet implement update, activate, deactivate and password reset. Also the token expiry date can't yet be specified. These things will need to be implemented as follow-ups when the features become available. Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
Remove the parts of the old `kci user` implementation that have now been implemented again with Click. Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
gctucker
force-pushed
the
kci-user-rework
branch
from
8c6736b
to
e7744b4
Compare
| from kernelci.cli import ( # pylint: disable=unused-import | ||
| docker as kci_docker, | ||
| user as kci_user, | ||
| ) |
There was a problem hiding this comment.
These are unused imports.
There was a problem hiding this comment.
Seems like it is required to register sub-commands. A comment about it would have been helpful.
There was a problem hiding this comment.
Yes it's quite implicit, which is nice in a way as it means no additional code is required but it's against the Zen of Python. I'll add some comments there.
| @kci_user.group(name='password') | ||
| def user_password(): | ||
| """Manage user passwords""" |
There was a problem hiding this comment.
After I added sub-commands to this group, it became kci user password reset-password.
I expected it to be kci password reset-password.
What are the benefits of adding groups?
There was a problem hiding this comment.
Well this was as per the proposed syntax on the GitHub discussion:
kernelci/kernelci-project#263 (comment)
It means all the user-related commands are grouped under kci user. There might be other passwords for other things, like kci storage password or anything else.
Implement parts of
kci userwith the new framework based on Click and remove the corresponding old implementation code.