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.

Sign up for GitHub

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

Jump to bottom

[API] list datasets #1119

Merged
merged 11 commits into from
Nov 28, 2024
Merged

[API] list datasets #1119

merged 11 commits into from
Nov 28, 2024

Conversation

jbygdell
Copy link
Collaborator

@jbygdell jbygdell commented Nov 5, 2024 •
edited
Loading

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
@jbygdell jbygdell force-pushed the feature/api-list-datasets branch from 6ca1edf to 12d2ef0 Compare November 5, 2024 14:21
MalinAhlberg
MalinAhlberg reviewed Nov 6, 2024
View reviewed changes
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 force-pushed the feature/api-list-datasets branch from 12d2ef0 to c7619d8 Compare November 7, 2024 08:52
@jbygdell jbygdell requested review from a team and MalinAhlberg November 7, 2024 08:52
MalinAhlberg
MalinAhlberg reviewed Nov 11, 2024
View reviewed changes
sda/cmd/api/api.md Outdated Show resolved Hide resolved
MalinAhlberg
MalinAhlberg previously approved these changes Nov 11, 2024
View reviewed changes
@jbygdell jbygdell force-pushed the feature/api-list-datasets branch from c7619d8 to 33de56f Compare November 11, 2024 12:39
MalinAhlberg
MalinAhlberg previously approved these changes Nov 12, 2024
View reviewed changes
pahatz
pahatz previously approved these changes Nov 13, 2024
View reviewed changes
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
@jbygdell jbygdell dismissed stale reviews from pahatz and MalinAhlberg via 50a6642 November 14, 2024 09:07
@jbygdell jbygdell force-pushed the feature/api-list-datasets branch from 33de56f to 50a6642 Compare November 14, 2024 09:07
MalinAhlberg
MalinAhlberg previously approved these changes Nov 15, 2024
View reviewed changes
@jbygdell jbygdell force-pushed the feature/api-list-datasets branch from 50a6642 to 8d31415 Compare November 18, 2024 11:07
MalinAhlberg
MalinAhlberg previously approved these changes Nov 19, 2024
View reviewed changes
nanjiangshu
nanjiangshu reviewed Nov 20, 2024
View reviewed changes
sda/cmd/api/api.md Outdated Show resolved Hide resolved
nanjiangshu
nanjiangshu reviewed Nov 20, 2024
View reviewed changes
sda/cmd/api/api.md Outdated Show resolved Hide resolved
nanjiangshu
nanjiangshu reviewed Nov 20, 2024
View reviewed changes
sda/cmd/api/api.md Outdated Show resolved Hide resolved
nanjiangshu
nanjiangshu reviewed Nov 20, 2024
View reviewed changes
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.

nanjiangshu
nanjiangshu approved these changes Nov 26, 2024
View reviewed changes
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!

@jbygdell jbygdell force-pushed the feature/api-list-datasets branch from 0b4ef1f to e35692a Compare November 27, 2024 11:09
@jbygdell jbygdell added this pull request to the merge queue Nov 28, 2024
Merged via the queue into main with commit 84b1534 Nov 28, 2024
27 checks passed
@jbygdell jbygdell deleted the feature/api-list-datasets branch November 28, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pahatz pahatz pahatz approved these changes

@nanjiangshu nanjiangshu nanjiangshu approved these changes

@MalinAhlberg MalinAhlberg Awaiting requested review from MalinAhlberg

Assignees

@jbygdell jbygdell

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jbygdell @MalinAhlberg @nanjiangshu @pahatz