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

Feature/database API #69

Merged
merged 17 commits into from
Jul 21, 2023
Merged

Feature/database API #69

merged 17 commits into from
Jul 21, 2023

Conversation

quanpython
Copy link
Collaborator

No description provided.

Copy link

@ReiHashimoto ReiHashimoto left a comment

Choose a reason for hiding this comment

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

LGTM

@quanpython quanpython linked an issue Jul 19, 2023 that may be closed by this pull request
studio/app/optinist/routers/expdb.py Outdated Show resolved Hide resolved
studio/app/optinist/routers/expdb.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@itutu-tienday itutu-tienday left a comment

Choose a reason for hiding this comment

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

@quanpython
I have added some remarks, please check them.

studio/app/optinist/routers/expdb.py Outdated Show resolved Hide resolved
studio/app/optinist/routers/expdb.py Show resolved Hide resolved
Copy link
Collaborator

@itutu-tienday itutu-tienday left a comment

Choose a reason for hiding this comment

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

@quanpython
I have added some remarks. Please check them.

)
query = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems that in some cases the actual number of acquisitions (less than 50) and the total of pagenation do not match.
-> Please check if the query and pagenation are working well together.
-> If you need to provide sample data, please ask us.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't reproduce it yet, please provide sample data

Copy link
Collaborator

@itutu-tienday itutu-tienday Jul 21, 2023

Choose a reason for hiding this comment

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

@quanpython
I will send sample data.
*API response results are also attached for reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@itutu-tienday I investigated the error and created a PR #73

studio/app/common/schemas/users.py Outdated Show resolved Hide resolved
@itutu-tienday itutu-tienday merged commit 37c664f into develop-main Jul 21, 2023
@itutu-tienday
Copy link
Collaborator

@quanpython CC: @reactplus-thom
There are some remaining tasks for this PR, but most of the points have been addressed, so I approve it once.
Please continue to investigate the following points in a separate PR.

sanglevinh pushed a commit that referenced this pull request Oct 16, 2023
ReiHashimoto pushed a commit that referenced this pull request Dec 11, 2023
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.

Database API実装①
3 participants