-
Notifications
You must be signed in to change notification settings - Fork 354
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
Bugfixes/align availables workspaces and set workspace operation #3263
Bugfixes/align availables workspaces and set workspace operation #3263
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #3263 +/- ##
==========================================
+ Coverage 90.73% 91.04% +0.30%
==========================================
Files 219 219
Lines 11673 11681 +8
==========================================
+ Hits 10592 10635 +43
+ Misses 1081 1046 -35
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, but in User
there are some inconsistencies as we're not always the current User
but e.g. the owner listing all the users, so workspaces
listing would fail, and I wouldn't update the SDK until the mock is fixed in a separate PR. But I agree with the missing type-hints, extended unit tests, and Argilla.__init__
fixes
…operation' of github.com:argilla-io/argilla into bugfixes/align-availables-workspaces-and-set-workspace-operation
Description
This PR adapts the client workflow to check available workspaces before the
rg.set_workspace
function. Since theset_workspace
is based onlist_workspaces
method, a code refactor has been applied to return only available workspaces for the connected user.@alvarobartt we need to align the
User.workspaces
,list_workspace
, andWorkspace.create
Closes #3262
Type of change
(Please delete options that are not relevant. Remember to title the PR according to the type of change)
Checklist