-
Notifications
You must be signed in to change notification settings - Fork 399
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
feat: create user with workspaces #3462
feat: create user with workspaces #3462
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #3462 +/- ##
===========================================
+ Coverage 90.13% 90.23% +0.09%
===========================================
Files 233 248 +15
Lines 12493 13235 +742
===========================================
+ Hits 11261 11943 +682
- Misses 1232 1292 +60
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
LGTM! Thanks 🤗
085abf1
to
2c51b72
Compare
# Description This PR fixes the issue (reported by @damianpumar), where the values of an already created response for a given record were not updated, even the server returned a `200`. The bug was introduced in #3462, where the `try: ... db.commit() except Exception: ... db.rollback()` where replaced with `async with db.begin_nested()` block. **Type of change** - [x] Bug fix (non-breaking change which fixes an issue) **How Has This Been Tested** The Docker image of this branch has been deployed in `dev.argilla.io`, where I was able to update the values of a response, refresh the page and see the updated values. **Checklist** - [ ] I added relevant documentation - [x] follows the style guidelines of this project - [x] I did a self-review of my code - [x] I made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK) (see text above) - [x] I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)
# Description This PR updates the API and the Python client so a list of workspaces names to which the user should be linked to can be provided. This is useful when creating a new user, as just one request will be needed, instead of the current behaviour, which requires one to create the user and another one to add the user to the workspace. In addition, the code blocks in which a commit or a rollback has to be done depending on wether an exception is raised have been updated to use the async context manager `async with session.begin_nested():` which automatically will commit if the block is executed without any problem or rollback if an exception is raised. **Type of change** - [x] New feature (non-breaking change which adds functionality) - [x] Improvement (change adding some improvement to an existing functionality) **How Has This Been Tested** I've been able to create a user with workspaces in a local development environment. Also, the unit tests regarding the user creation have been updated. **Checklist** - [ ] I added relevant documentation - [x] follows the style guidelines of this project - [x] I did a self-review of my code - [x] I made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK) (see text above) - [x] I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)
# Description This PR fixes the issue (reported by @damianpumar), where the values of an already created response for a given record were not updated, even the server returned a `200`. The bug was introduced in #3462, where the `try: ... db.commit() except Exception: ... db.rollback()` where replaced with `async with db.begin_nested()` block. **Type of change** - [x] Bug fix (non-breaking change which fixes an issue) **How Has This Been Tested** The Docker image of this branch has been deployed in `dev.argilla.io`, where I was able to update the values of a response, refresh the page and see the updated values. **Checklist** - [ ] I added relevant documentation - [x] follows the style guidelines of this project - [x] I did a self-review of my code - [x] I made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK) (see text above) - [x] I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)
Description
This PR updates the API and the Python client so a list of workspaces names to which the user should be linked to can be provided. This is useful when creating a new user, as just one request will be needed, instead of the current behaviour, which requires one to create the user and another one to add the user to the workspace.
In addition, the code blocks in which a commit or a rollback has to be done depending on wether an exception is raised have been updated to use the async context manager
async with session.begin_nested():
which automatically will commit if the block is executed without any problem or rollback if an exception is raised.Type of change
How Has This Been Tested
I've been able to create a user with workspaces in a local development environment. Also, the unit tests regarding the user creation have been updated.
Checklist