-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support api-key and password as parameter when using users.create_default task #2516
Support api-key and password as parameter when using users.create_default task #2516
Conversation
…iables to be used when creating default user
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feat/add-basic-database-support #2516 +/- ##
===================================================================
+ Coverage 92.74% 93.58% +0.83%
===================================================================
Files 167 163 -4
Lines 8271 8195 -76
===================================================================
- Hits 7671 7669 -2
+ Misses 600 526 -74
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@@ -30,17 +31,17 @@ def create_default(quiet: bool): | |||
first_name="", | |||
username=DEFAULT_USERNAME, | |||
role=UserRole.admin, | |||
api_key=DEFAULT_API_KEY, | |||
password_hash=accounts.hash_password(DEFAULT_PASSWORD), | |||
api_key=settings.api_key, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these settings are used only for the argilla task, I would prefer using a click.option instead, and calling the tasks:
python -m argilla.tasks.users.create_default --password $MY_PASS --api-key $MY_API_KEY
For me, it's a bit weird having app env vars used only on this task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree but then users.create_default
is useless because we already have users.create
(and it will have the possibility of setting the api key, that is necessary for the team user of the QuickStart docker image).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I Agree. Also, in docs we can just say:
"Create the default user by typing:
argilla users create --username argilla --password 1234 --api-key argilla.apiKey --role admin
EDIT
Indeed is not possible since the password constraints. And I would like to keep this constraints for the common user creation command
…ated default username before create it
This PR includes the following changes:
--api-key
and--password
parameters toargilla.tasks.users.create_default
.argilla.tasks.users.create_default
checks if there is a user using default username before creating it and showing a message in such case (but not raising an error).