SQLAlchemy Prep: Surge on Query Updates

[mjones-oddball]

User Story - Business Need

A pre-requisite to #1225 (SQLAlchemy Upgrade) is to update all the queries. We'd like a team surge on this as there will be many folders and files to update within the API codebase.

• Ticket is understood, and QA has been contacted (if the ticket has a QA label).

User Story(ies)

As a platform
I want to keep my packages up to date
So that we avoid vulnerabilities and can use the latest functionality

Additional Info and Resources

• sqlalchemy 2.0 query syntax
• migration from 1.4 to 2.0 ORM usage table

Starter files to jump into:

Location	Query count
app/celery/nightly_tasks.py	1
app/celery/scheduled_tasks.py	1
app/dao/init.py	1
app/dao/provider_rates_dao.py	1

• Update this spreadsheet when taking a new file

Current workload:
Private Zenhub Image

1436 has cleared around 60 of those queries.

Engineering Checklist

• Jump on a call and divvy up folders/files

Acceptance Criteria

• regex search for \.query(\.|\() returns nothing
  • that would be things matching .query. or .query(
  • Skipped tests will have the queries removed
    - [ ] All select queries use read-only where appropriate Ended up being too much work just to convert everything. Read-only will be setup as-needed.

QA Considerations

• regression passes on this branch in Dev
• regression in Perf

[mjones-oddball]

Hey team! Please add your planning poker estimate with Zenhub @cris-oddball @edDocMe360 @EvanParish @k-macmillan @kalbfled @ldraney @nikolai-efimov

[k-macmillan]

Team meeting times have been planned for Monday and Tuesday 1:30 to 3:30 Eastern.

[k-macmillan]

We're all on the same page for updating:

• How we will address the queries
• How to see the new query is the same
• How to test changes
• Branch management

We are tracking progress with a document.

Most of the first few hours was just getting everyone setup and on the same page. Should be able to cruise now.

[k-macmillan]

All queries have been updated. We are working through addressing unit test failures then we will deploy and run the regression.

[k-macmillan]

Reviewed all PR comments and came to team consensus on each. Team is updating the PR based on the comments.

[k-macmillan]

Friday and today we addressed the remaining comments.

• All tests pass
• Deployed
• All regression tests pass

Will deploy to perf, regression test, then deploy to staging.

[k-macmillan]

Perf deployed, passes regression. Staging deployed, passed regression. Going to let it sit.

[k-macmillan]

Put this in QA testing for now. It is not merged. Giving it some time in staging to see if anything goes sideways.

[k-macmillan]

Dug through all logs over the last 24 hours in staging.
The delete_notifications_older_than_retention_by_type method appears to have a warning.

/app/app/dao/notifications_dao.py:568: SAWarning: Coercing Subquery object into a 
select() for use in IN(); please pass a select() construct explicitly

The only other part to check are the stats query results. Would like to work with @cris-oddball, @EvanParish, and @mjones-oddball when they get back to validate the stats.

[mjones-oddball]

In Staging I sent the following tests. Will check BQ tomorrow to confirm aggregation is correct in stats and billing.

[k-macmillan]

Another bug appears to exist between a Twilio notification being added to the DB and a lookup on that notification. Requires further investigation.

[mjones-oddball]

Cris and I confirmed stats and billing data looked good

[k-macmillan]

Updated with master (python 3.10/alpine upgrade).

Wrote a new method for the delivery status and wrote 44 tests (2 methods with 27 and 17 parameterizations) for the new method.

[cris-oddball]

PR approved and merged; master sent up to Perf. Just going to let it bake there over the weekend after running a regression.

[cris-oddball]

Regression passes, expected data in BQ, no errors seen in logs over the weekend. Given the testing this went through in staging, calling it good and closing ticket.