-
Notifications
You must be signed in to change notification settings - Fork 93
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
Move IRS to Actor model #1000
Move IRS to Actor model #1000
Conversation
Millennium PR! 💯0 |
Codecov Report
@@ Coverage Diff @@
## master #1000 +/- ##
==========================================
+ Coverage 93.27% 93.33% +0.06%
==========================================
Files 287 287
Lines 3701 3677 -24
Branches 67 72 +5
==========================================
- Hits 3452 3432 -20
+ Misses 249 245 -4
Continue to review full report at Codecov.
|
I'm not seeing the #979 error occur on this branch. Can someone else test/confirm this? To clarify: when viewing the Postgres |
case class MsaMap( | ||
msas: ListMap[String, Msa] = ListMap[String, Msa]() | ||
) { | ||
def +(lar: LoanApplicationRegister): MsaMap = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How many LAR records do we usually see here? ListMap
is a collection that is only suitable for small number of records. Do you need the extra properties of a ListMap
or would an immutable Map
be sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the IRS, since the rows are by MSA/MD (plus a total row), there should be no more than 1000 rows. Not sure if that is considered small.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My original concern was related to ordering and pagination. Before I switched to storing a Seq
in the ValidationStats, I was worried about relying on the consistency of the iteration order through a Map
. ListMap
guarantees a sorted order based on insertion, but that property is no longer necessary. I'll make the switch back to an immutable Map
.
@nickgrippin I'll check on #979 and try to confirm that it's fixed with this update. |
Closes #984