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.

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: add workspace_id param to GET /api/v1/me/datasets #3727

Merged
merged 13 commits into from
Sep 7, 2023

Conversation

alvarobartt
Copy link
Member

@alvarobartt alvarobartt commented Sep 7, 2023

Description

This PR adds the workspace_id param to GET /api/v1/me/datasets so that the workspace filtering when listing FeedbackTask datasets is applied in the API-side, as well as making sure that no local filters are applied e.g. FeedbackDataset.list(workspace=...)

Closes #3726

Type of change

  • New feature (non-breaking change which adds functionality)
  • Improvement (change adding some improvement to an existing functionality)

How Has This Been Tested

  • Add tests for GET /api/v1/me/datasets using the workspace_id param, including also the updated policies for non-owner users
  • Add tests for list_datasets in the Python SDK using the workspace_id arg
  • Add tests for FeedbackDataset.list in the Python client using the workspace arg

Checklist

  • I added relevant documentation
  • follows the style guidelines of this project
  • I did a self-review of my code
  • I made corresponding changes to the documentation
  • 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 (see text above)
  • I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)

@alvarobartt alvarobartt added type: enhancement Indicates new feature requests client labels Sep 7, 2023
@alvarobartt alvarobartt added this to the v1.16.0 milestone Sep 7, 2023
@alvarobartt alvarobartt self-assigned this Sep 7, 2023
@alvarobartt alvarobartt marked this pull request as ready for review September 7, 2023 11:09
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Patch coverage is 95.65% of modified lines.

Files Changed Coverage
src/argilla/client/feedback/dataset/mixins.py 66.66%
src/argilla/client/sdk/v1/datasets/api.py 100.00%
src/argilla/server/apis/v1/handlers/datasets.py 100.00%
src/argilla/server/policies.py 100.00%

📢 Thoughts on this report? Let us know!.

@github-actions
Copy link

github-actions bot commented Sep 7, 2023

The URL of the deployed environment for this PR is https://argilla-quickstart-pr-3727-ki24f765kq-no.a.run.app

Co-authored-by: Gabriel Martín Blázquez <[email protected]>
Comment on lines +81 to +85
workspace = await WorkspaceFactory.create()
dataset = await DatasetFactory.create(workspace=workspace)
user = await UserFactory.create(role=role, workspaces=[dataset.workspace] if with_workspace else [])

another_workspace = await WorkspaceFactory.create()
Copy link
Member

Choose a reason for hiding this comment

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

(Just a comment) we should start thinking about a seed script to feed integration test data in a different way

Copy link
Member Author

Choose a reason for hiding this comment

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

With "a different way" do you mean without using the factories? Ideally we can define a function on each test suite (we are already doing that in some places) to call a self function for e.g. create a dataset (with different args), to call that function instead of explicit calls to the factories

@alvarobartt alvarobartt merged commit fb92ca2 into develop Sep 7, 2023
@alvarobartt alvarobartt deleted the feat/list-datasets-using-workspace branch September 7, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add workspace_id to GET /api/v1/me/datasets
3 participants