Skip to content
This repository has been archived by the owner on Feb 20, 2024. It is now read-only.

Add more user management #115

Merged
merged 23 commits into from
Jun 11, 2019
Merged

Add more user management #115

merged 23 commits into from
Jun 11, 2019

Conversation

nginyc
Copy link
Owner

@nginyc nginyc commented Jun 6, 2019

Fixes #111

Before testing, since Rafiki's v0.1.0 Docker images have not been published to Docker Hub, will need to run source .env.sh, then bash scripts/build_images.sh for every participating machine.

  • Admins can ban users
  • Admins can list all users
  • Admins cannot ban other admins (only superadmins can do this)
  • Admins cannot ban themselves
  • Admins cannot create a user with an invalid type
  • Add documentation on new user APIs

@nginyc nginyc requested a review from wild-flame June 6, 2019 22:40
with admin:
return jsonify(admin.create_users_with_csv(**params))

@app.route('/users', methods=['POST'])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not common to create users using a csv file. Typically, we add users one by one. But we need to get all users for the front-end.
Have you compared users vs user for the operations against a single user? Both are fine, but we need to be consistent with other APIs. E.g., model vs models, trial vs trials.

Copy link
Owner Author

@nginyc nginyc Jun 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. I will be removing this API

@nginyc
Copy link
Owner Author

nginyc commented Jun 7, 2019

Changed from deleting user to banning user to avoid complication of having to consider cascade-deleting all associated train jobs / trials / inference jobs / models under that user.

@wild-flame
Copy link
Collaborator

wild-flame commented Jun 10, 2019

Can add to documentation:

for sudo users need to run

sudo -E bash scripts/build_images.sh after run source env.sh

according to https://stackoverflow.com/questions/8633461/how-to-keep-environment-variables-when-using-sudo to preserve the environment variable

@nginyc
Copy link
Owner Author

nginyc commented Jun 10, 2019

Thanks @wild-flame for the feedback. I've added that to the documentation.

@wild-flame wild-flame merged commit c4b2f2b into v0.1.0 Jun 11, 2019
@nginyc nginyc deleted the delete-users branch June 11, 2019 17:20
@nginyc nginyc mentioned this pull request Jun 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants