-
Notifications
You must be signed in to change notification settings - Fork 40
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
15257 - Fix statements serialization issue #1115
Conversation
…sqlalchemy in the future.
Codecov Report
@@ Coverage Diff @@
## main #1115 +/- ##
==========================================
+ Coverage 91.54% 91.62% +0.07%
==========================================
Files 153 163 +10
Lines 9842 10947 +1105
==========================================
+ Hits 9010 10030 +1020
- Misses 832 917 +85
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…hedule, we can get that from the invoice instead. More cleanup.
Still needs to be tested, but seems to pass all the unit tests. |
…ement_serialization
…ement_serialization # Conflicts: # pay-api/src/pay_api/models/payment.py # pay-api/src/pay_api/services/payment.py
…imized. Add in missing bcol_Account field.
I've done some comparison on the payload versus marshmallow, added a fix for removing None fields. Seems to be ok. 50,000~ invoices took 42s second total to download a CSV, with two like filters It took 1.3 minutes to download a CSV with the details search parameter entered (worst performance case). Date filter range only: Note our limit is 60,000 - and some of the more advanced filters could take longer (details for example, details will be fixed in the future when we move the column from jsonb to it's own table). I've upped the gunicorn timeout to 100s just incase. We'll have to update the route timeouts to 100s as well. I checked all the columns in the master transaction list, they seem to be all populating: |
…r doesn't work). Put in note regarding payment accounts needing one row.
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.
Nice work 👍
One optional comment. If you want we can make that change afterwards as its currently not using the config file anyways
# Conflicts: # pay-api/docker-entrypoint.sh
Kudos, SonarCloud Quality Gate passed!
|
@kialj876 |
Issue #:
bcgov/entity#15257
Description of changes:
Use cattrs instead of pydantic. Minor fix in migration for upgrading sqlalchemy in the future.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-pay license (Apache 2.0).