Skip to content

FINERACT-857: Fixing scheduler integration tests and migrating to executeAndAwaitJob#1049

Merged
vorburger merged 2 commits intoapache:developfrom
ptuomola:FINERACT-857
Jun 14, 2020
Merged

FINERACT-857: Fixing scheduler integration tests and migrating to executeAndAwaitJob#1049
vorburger merged 2 commits intoapache:developfrom
ptuomola:FINERACT-857

Conversation

@ptuomola
Copy link
Contributor

@ptuomola ptuomola commented Jun 13, 2020

Description

Fixed two bugs that were triggered by scheduler integration tests:

  • Invalid query in SavingsAccountCharge (missing parameters)
  • Lazy loading not working for m_savings_account.group_id due to transaction boundary

Also changed triggering of jobs to use executeAndAwaitJob() and fixed logging for exceptions jobs

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide

@ptuomola
Copy link
Contributor Author

OK - looks like turning off lazy loading for group means that we have more than 61 (!) tables in a join in RecurringDeposit, which then breaks... so need to find a way to get the lazy loading to work after all...

@ptuomola ptuomola marked this pull request as draft June 13, 2020 04:18
@ptuomola ptuomola marked this pull request as ready for review June 13, 2020 05:12
@ptuomola ptuomola force-pushed the FINERACT-857 branch 2 times, most recently from 1d3afee to ddef698 Compare June 13, 2020 06:18
Copy link
Member

@vorburger vorburger left a comment

Choose a reason for hiding this comment

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

Cool!! You are amazing. Some minor feedback..

PS: I'll search in JIRA for the open real bugs you have also fixed here.

Copy link
Member

@vorburger vorburger left a comment

Choose a reason for hiding this comment

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

LGTM! Would love to merge this ASAP.

@ptuomola if you are on and want to do this ASAP, it may avoid new conflicts for #943 which I'd like to do in the coming hours. If I don't hear from you, perhaps I'll even exceptionally resolve the conflict myself just to be able to merge this sooner rather then later - I'm sure you wouldn't mind.

@vorburger
Copy link
Member

Actually the merge conflict here was due to #1020 and trivial to resolve - I have, for once, just used GitHub's Web UI ... if I didn't screw it up, hopefully this will pass the build and I'll merge it.

@vorburger vorburger merged commit e8ecd79 into apache:develop Jun 14, 2020
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