Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add new CLI command to edit multiple fields at once #8075

Merged
merged 41 commits into from
Oct 25, 2023
Merged

feat: add new CLI command to edit multiple fields at once #8075

merged 41 commits into from
Oct 25, 2023

Conversation

AmanuelAaron
Copy link
Contributor

@AmanuelAaron AmanuelAaron commented Oct 4, 2023 •
edited
Loading

Description

Add new CLI command det user edit that take optional inputs to edit different user fields in one command.
Following fields are supported:

  • Username
  • Display Name
  • Admin
  • Remote
  • Active

Test Plan

Test added to e2e_tests/tests/cluster/test_users.py

Manual testing can be done in the command line after running make devcluster. Check local db to verify changed fields.

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

DET-9880

@cla-bot cla-bot bot added the cla-signed label Oct 4, 2023
@netlify
Copy link

netlify bot commented Oct 4, 2023 •
edited
Loading

✅ Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 7b83757
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/653955be4cb3b9000819c9ff

@determined-ci determined-ci added the documentation Improvements or additions to documentation label Oct 5, 2023
@varlogtim
Copy link
Member

Awesome! Thanks Aaron!

  1. Could you also add the ability to specify the link-with-agent-user options too? (my thinking was that we should be able to modify anything with a single line ... if not, this is fine :) )
  2. Could you add tests too? e2e_tests/tests/cluster/test_users.py

@AmanuelAaron AmanuelAaron marked this pull request as ready for review October 6, 2023 13:28
@AmanuelAaron AmanuelAaron requested review from a team as code owners October 6, 2023 13:28
maxrussell
maxrussell requested changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@maxrussell maxrussell left a comment

Choose a reason for hiding this comment

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

Looks really good! Couple things, but this looks close to me.

e2e_tests/tests/cluster/test_users.py Outdated Show resolved Hide resolved
harness/determined/cli/user.py Outdated Show resolved Hide resolved
harness/determined/cli/user.py Outdated Show resolved Hide resolved
harness/determined/common/experimental/user.py Outdated Show resolved Hide resolved
tara-det-ai
tara-det-ai reviewed Oct 10, 2023
View reviewed changes
Copy link
Member

@tara-det-ai tara-det-ai left a comment

Choose a reason for hiding this comment

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

suggested edit

@cla-bot
Copy link

cla-bot bot commented Oct 10, 2023

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @AmanuelAaron on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla.

After we approve your CLA, we will update the contributors list (private) and comment @cla-bot[bot] check to rerun the check.

@cla-bot cla-bot bot removed the cla-signed label Oct 10, 2023
@cla-bot
Copy link

cla-bot bot commented Oct 10, 2023

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @AmanuelAaron on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla.

After we approve your CLA, we will update the contributors list (private) and comment @cla-bot[bot] check to rerun the check.

@cla-bot cla-bot bot added the cla-signed label Oct 10, 2023
@cla-bot
Copy link

cla-bot bot commented Oct 10, 2023

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @AmanuelAaron on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla.

After we approve your CLA, we will update the contributors list (private) and comment @cla-bot[bot] check to rerun the check.

@cla-bot cla-bot bot removed the cla-signed label Oct 10, 2023
@cla-bot cla-bot bot added the cla-signed label Oct 10, 2023
@maxrussell maxrussell self-requested a review October 11, 2023 19:05
wes-turner
wes-turner reviewed Oct 16, 2023
View reviewed changes
harness/determined/cli/user.py Outdated Show resolved Hide resolved
harness/determined/cli/user.py Outdated Show resolved Hide resolved
harness/determined/common/declarative_argparse.py Show resolved Hide resolved
harness/determined/common/experimental/user.py Outdated Show resolved Hide resolved
harness/determined/cli/user.py Outdated Show resolved Hide resolved
harness/determined/cli/user.py Show resolved Hide resolved
harness/determined/cli/user.py Outdated Show resolved Hide resolved
@determined-ci determined-ci requested a review from a team October 17, 2023 15:32
varlogtim
varlogtim approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@varlogtim varlogtim left a comment

Choose a reason for hiding this comment

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

Thanks!

@AmanuelAaron AmanuelAaron requested review from a team as code owners October 25, 2023 17:12
@determined-ci
Copy link
Collaborator

Docsite preview being generated for this PR.
You can (eventually) find the generated docsite here.

1 similar comment
@determined-ci
Copy link
Collaborator

Docsite preview being generated for this PR.
You can (eventually) find the generated docsite here.

@determined-ci determined-ci requested a review from a team October 25, 2023 17:50
@AmanuelAaron AmanuelAaron merged commit c52ef71 into main Oct 25, 2023
71 of 82 checks passed
@AmanuelAaron AmanuelAaron deleted the cli_edit_user_displayname branch October 25, 2023 18:24
@dannysauer dannysauer added this to the 0.26.3 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tara-det-ai tara-det-ai tara-det-ai left review comments

@wes-turner wes-turner wes-turner approved these changes

@maxrussell maxrussell maxrussell approved these changes

@varlogtim varlogtim varlogtim approved these changes

Assignees
No one assigned
Labels
cla-signed documentation Improvements or additions to documentation User-facing API Change
Projects
None yet
Milestone
0.26.3
Development

Successfully merging this pull request may close these issues.
7 participants
@AmanuelAaron @varlogtim @determined-ci @maxrussell @wes-turner @tara-det-ai @dannysauer