Skip to content

Fixup CSG reports to batch-send; Tweak services definition for better…#4699

Open
ZachCMP wants to merge 5 commits intostablefrom
zparks/9-12-24-fixup-csg-reports
Open

Fixup CSG reports to batch-send; Tweak services definition for better…#4699
ZachCMP wants to merge 5 commits intostablefrom
zparks/9-12-24-fixup-csg-reports

Conversation

@ZachCMP
Copy link
Contributor

@ZachCMP ZachCMP commented Sep 12, 2024

… fallback

Please squash merge this PR

Description

  • Updates CSG Engage export to batch send households
  • Fixup service report component to have a better fallback if it can't create the service type (seems like it never can)

Type of change

  • New feature (adds functionality)

Checklist before requesting review

  • I have performed a self-review of my code
  • I have run the code that is being changed under ideal conditions, and it doesn't fail
  • My code includes comments and/or descriptive variable names to help other engineers understand the intent (or not applicable)
  • My code follows the style guidelines of this project (rubocop)
  • I have updated the documentation (or not applicable)
  • If it's not obvious how to test this change, I have provided testing instructions in this PR or the related issue

@ZachCMP ZachCMP requested a review from eanders September 12, 2024 21:31
Copy link
Contributor

@eanders eanders left a comment

Choose a reason for hiding this comment

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

@ZachCMP I think this looks good, as long as the receiving end can handle receiving partial data sets. With the previous version it would fail atomically, now it might fail after some number of successful batches, but that's probably fine.

Is there any way to batch load data? I think part of what makes it slow is that it is pulling a query for each client from the database.

@ZachCMP
Copy link
Contributor Author

ZachCMP commented Sep 13, 2024

@eanders Grant confirmed that the API can accept partial uploads. Ideally we shouldn't send duplicate records, but the API will handle it by just returning an error for any duplicates. I think this will be better, since I think the hanging is caused by trying to serialize and upload so many records at once. And yeah, I agree it'd be best to batch load it. One consideration is that we have to load all the HOH enrollments first, then all the enrollment records for that household, including client records, services, incomes, etc. If you have a good strategy for pre-loading all the information, I'd be happy to implement it.

@eanders
Copy link
Contributor

eanders commented Sep 13, 2024

Thanks @ZachCMP. What we've done for batching in the past is to pluck the HouseholdIDs, and then loop over batches of those pulling all related data. Sometimes those batches need to be a bit smaller (say 250 instead of 1,000) since they bring along multiple clients, but it's worked pretty well.

@ZachCMP
Copy link
Contributor Author

ZachCMP commented Sep 16, 2024

@eanders I adjusted the batching to more or less do what you describe. Take a look and let me know if you like that better. Also Grant asked that we just send an integer for the income amount field, so I made that change as well. Let me know if you need anything else from me to get this deployed.

Base automatically changed from release-133 to stable September 30, 2024 13:13
@eanders eanders changed the base branch from stable to release-137 October 16, 2024 13:10
Base automatically changed from release-137 to stable October 30, 2024 13:00
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.

2 participants