OpenFn / unicef-cambodia

UNICEF Cambodia - Primero Interoperability
https://openfn.github.io/unicef-cambodia/
1 stars 2 forks source link

Add pagination for oscar referrals #131

Open mtuchi opened 2 months ago

mtuchi commented 2 months ago

Description

Add ability to paginate oscar cases. Since we are fetching cases with referrals i have limit the pagination to 20 cases per request because in each 20 cases we are making 20 request to fetch case referrals.

These changes have been test using the following versions

╭─────────────────────────────────────────────╮
│ ◲ ◱  @openfn/core#v1.5.1 (Node.js v18.12.1) │
│ ◳ ◰         @openfn/language-primero@2.11.7 │
╰─────────────────────────────────────────────╯

Ref #130

josephjclark commented 2 months ago

@mtuchi this branch is in conflict with master

josephjclark commented 2 months ago

@mtuchi I'm not sure I understand the reasoning behind fetching 20 cases at a time.

Whether you fetch 20 or 1000 at once, you're still going to make a separate request for each individual case. So what benefit does a smaller batch have?

josephjclark commented 2 months ago

@mtuchi also this problem was true on the original version - the base branch will still send out a new request for every referral. All we've done in this PR is is make MORE Api calls. Significantly more if we're pulling down ~1000 records.

mtuchi commented 2 months ago

@josephjclark withReferrals option is the reason for creating a small batch because it will fetch referrals for each case. If you have 20 cases it will make 20 request to fetch referrals. So if we use 1000 and get 1000 cases back then it will make 1000 request to fetch referrals.

Making a small batch can help with retry incase of failure because you can adjust the currentPage and retry from there

josephjclark commented 2 months ago

Ok, the debugging argument seems valid. I'm just interested in how much slower this change is going to make it.

Again, in the v1 implementation, it just fetched everything and then fetched referrals one at a time and extra debugging wasn't needed then :shrug:

Maybe a bigger page, perhaps 100 cases, would be a better compromise?

One thing I would recommend is that you add logging in the job code at the start and end of each page

mtuchi commented 1 month ago

@josephjclark fixed the git conflict I have updated the perPage to 100 and add logs before and after each getCases