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

Add start_date, end_date & pagination parameters #324

Closed
wants to merge 2 commits into from

Conversation

RonanMorgan
Copy link
Collaborator

@RonanMorgan RonanMorgan commented May 3, 2024

In order to facilitate data science management I have refactored the endpoint "fetch_media" : I have added a start_date, an end_date to gather images during this time period and a pagination parameter in order to get images 10 by ten

=> still need to adapt pyro-client and to add test in the client part

@RonanMorgan RonanMorgan self-assigned this May 3, 2024
@RonanMorgan RonanMorgan marked this pull request as draft May 3, 2024 16:20
Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 94.27%. Comparing base (767be30) to head (571e3c5).
Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
src/app/api/endpoints/media.py 88.23% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
- Coverage   94.54%   94.27%   -0.27%     
==========================================
  Files          63       66       +3     
  Lines        1594     1695     +101     
==========================================
+ Hits         1507     1598      +91     
- Misses         87       97      +10     
Flag Coverage Δ
client 90.58% <ø> (?)
unittests 94.47% <91.66%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RonanMorgan
Copy link
Collaborator Author

RonanMorgan commented May 13, 2024

some thoughts / questions :
=> this PR is useless, I should I have implemented the pagination code in the client part, not in the API part, no ?
=> Can I add the boto dependency in the client ?
=> instead of using fetch_media I should have used get_media_url
=> Mateo needs Alerts + URL

@frgfm
Copy link
Member

frgfm commented Jun 23, 2024

I think the pagination idea is great for all the "fetch" routes, perhaps we should revisit this for all endpoints. Some rebasing is needed first though 😅

@RonanMorgan
Copy link
Collaborator Author

Yes I will go back to this pagination after the end of the big refactor :)
I think I will create a brand new PR

@RonanMorgan RonanMorgan deleted the rs/refactor-fetch-endpoint branch October 29, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants