[FEATURE]: Adding Functionality To Update Users #5615
Conversation
sean-hickey-wf
changed the title
There was a problem hiding this comment.
Great! Thanks @sean-hickey-wf !!!
Could you please update the image used in the SDK integration tests to run them with the new changes?
argilla-server:
image: argilladev/argilla-hf-spaces:pr-5615
There was a problem hiding this comment.
Hi @sean-hickey-wf thank you very much for your contribution ❤️ . I have added some small comments in case you want to apply them.
|
@frascuchon @jfcalvo thanks for the review! Will make those changes tonight or tomorrow 😄 |
|
@sean-hickey-wf, let me know if you need help with this PR |
@frascuchon @jfcalvo sorry this took so long, was swamped in work! I've made the changes requested but I having trouble getting the env set up to make sure the tests are passing. If one of ye could just check that everything looks sensible and make sure the tests pass we should be good to go! |
argilla-server/tests/unit/api/handlers/v1/users/test_update_user.py
Outdated
Show resolved
Hide resolved
argilla-server/tests/unit/api/handlers/v1/users/test_update_user.py
Outdated
Show resolved
Hide resolved
argilla-server/tests/unit/api/handlers/v1/users/test_update_user.py
Outdated
Show resolved
Hide resolved
argilla-server/tests/unit/api/handlers/v1/users/test_update_user.py
Outdated
Show resolved
Hide resolved
argilla-server/tests/unit/api/handlers/v1/users/test_update_user.py
Outdated
Show resolved
Hide resolved
argilla-server/tests/unit/api/handlers/v1/users/test_update_user.py
Outdated
Show resolved
Hide resolved
@frascuchon sorry I meant to come back to this and fix it this week! Thanks for pushing those changes. Patches make more sense than Puts 👍 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5615 +/- ##
===========================================
+ Coverage 91.95% 91.98% +0.03%
===========================================
Files 161 161
Lines 6750 6776 +26
===========================================
+ Hits 6207 6233 +26
Misses 543 543
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: José Francisco Calvo <[email protected]>
…hickey-wf/argilla into feat/add-update-user-functionality
|
@jfcalvo @frascuchon thank you for helping finish this off! |
# Description <!-- Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. --> This PR resolves some errors found after changes included in #5615. **Type of change** <!-- Please delete options that are not relevant. Remember to title the PR according to the type of change --> - Bug fix (non-breaking change which fixes an issue) **How Has This Been Tested** <!-- Please add some reference about how your feature has been tested. --> **Checklist** <!-- Please go over the list and make sure you've taken everything into account --> - I added relevant documentation - I followed the style guidelines of this project - I did a self-review of my code - I made corresponding changes to the documentation - I confirm My changes generate no new warnings - I have added tests that prove my fix is effective or that my feature works - I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)
Description
Argilla offers the ability to create and delete users but not the ability to update a User object after it has been created. For example, if we want to update the Role of a user after they have been created (from annotator to admin for example), this is not possible without deleting and recreating the User.
This PR adds an update endpoint to the FastAPI server and also the convenience of doing this through the python sdk also
Closes #<issue_number>
Type of change
How Has This Been Tested
Tests have been added at both the server and SDK level to ensure that the update method is working as expected
Checklist