Page MenuHomePhabricator

[SQLite] Add a new column to outbound_p2p_messages table
ClosedPublic

Authored by tomek on Jul 15 2024, 3:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 10, 5:29 AM
Unknown Object (File)
Wed, Sep 25, 7:54 PM
Unknown Object (File)
Sep 15 2024, 10:07 AM
Unknown Object (File)
Sep 15 2024, 10:05 AM
Unknown Object (File)
Sep 1 2024, 8:43 AM
Unknown Object (File)
Aug 28 2024, 7:30 PM
Unknown Object (File)
Aug 28 2024, 6:48 AM
Unknown Object (File)
Aug 28 2024, 3:53 AM
Subscribers

Details

Summary

Add a field that marks messages as being subjects to automatic retries.

https://linear.app/comm/issue/ENG-8729/automatic-retries-for-non-composable-dms

Test Plan

Open an existing app and check if the migration doesn't throw. Start a fresh app and check if the DB creation succeeds. Further testing is performed later in the stack.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2521–2522

These changes were made by clang-format-all

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 15 2024, 3:43 AM
Harbormaster failed remote builds in B30318: Diff 42279!
tomek requested review of this revision.Jul 15 2024, 3:54 AM
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
689

This column name comes across as "past tense", basically indicating whether this message has been automatically retried. But from the diff description, it seems like it's more about whether the message CAN be automatically retried. Should we consider something like "can_be_automatically_retried" or "supports_automatic_retry" or "supports_auto_retry"?

812

Nit: typo (missing space)

kamil added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
689

agree with that

This revision is now accepted and ready to land.Jul 16 2024, 3:08 AM

Rename column

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
689

Makes sense - renamed to supports_auto_retry

Heads-up that I landed a different migration 48 in D12759, so you'll have to rebase this one. Sorry about that!