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

[API] list datasets #1119

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[API] list datasets #1119

wants to merge 11 commits into from

Conversation

jbygdell
Copy link
Collaborator

@jbygdell jbygdell commented Nov 5, 2024

Description
This PR adds the functionality to list datasets with their status and date.

  • Admins can list all datasets as well as datasets with files belonging to a specific user.
    • /datasets/list
    • /datasets/list/USERNAME
  • Submission users can list datasets where they uploaded the files that are part of the dataset.
    • /datasets

@jbygdell jbygdell requested a review from a team November 5, 2024 14:07
@jbygdell jbygdell self-assigned this Nov 5, 2024
sda/cmd/api/api.go Show resolved Hide resolved
sda/cmd/api/api.go Outdated Show resolved Hide resolved
sda/cmd/api/api.md Outdated Show resolved Hide resolved
sda/cmd/api/api.md Outdated Show resolved Hide resolved
sda/internal/database/database.go Outdated Show resolved Hide resolved
@jbygdell jbygdell requested review from a team and MalinAhlberg November 7, 2024 08:52
sda/cmd/api/api.md Outdated Show resolved Hide resolved
MalinAhlberg
MalinAhlberg previously approved these changes Nov 11, 2024
MalinAhlberg
MalinAhlberg previously approved these changes Nov 12, 2024
pahatz
pahatz previously approved these changes Nov 13, 2024
Copy link
Contributor

@pahatz pahatz left a comment

Choose a reason for hiding this comment

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

Looks great!

sda/internal/database/db_functions.go Show resolved Hide resolved
MalinAhlberg
MalinAhlberg previously approved these changes Nov 15, 2024
MalinAhlberg
MalinAhlberg previously approved these changes Nov 19, 2024
sda/cmd/api/api.md Outdated Show resolved Hide resolved
sda/cmd/api/api.md Outdated Show resolved Hide resolved
sda/cmd/api/api.md Outdated Show resolved Hide resolved
Copy link
Contributor

@nanjiangshu nanjiangshu left a comment

Choose a reason for hiding this comment

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

Looks good except for some small issues.

In addition, I always got null when accessing the endpoint /datasets by

 curl    -H "Authorization: Bearer $token" -X GET "http://localhost:8090/datasets" 

Even after manually uploaded a file and ingested and created a dataset.

@jbygdell jbygdell force-pushed the feature/api-list-datasets branch 2 times, most recently from a0089a1 to 4e071c9 Compare November 22, 2024 13:30
@jbygdell jbygdell requested a review from a team November 22, 2024 13:35
@MalinAhlberg
Copy link
Contributor

I find it a bit hard to figure out what has changed recently.
All endpoints are called datasets now, not dataset (as stated in the PR description)?
Is there any fix for what Nanjiang reported?

@jbygdell
Copy link
Collaborator Author

I find it a bit hard to figure out what has changed recently. All endpoints are called datasets now, not dataset (as stated in the PR description)? Is there any fix for what Nanjiang reported?

Updated the PR description to match the code.
Nanjangs issue is no issue, it's actually covered in the tests.

Copy link
Contributor

@nanjiangshu nanjiangshu left a comment

Choose a reason for hiding this comment

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

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants