Allow for messages to be enqueued to dynamoDB when the
device is not available. Also, deliver messages once a connection with
said device becomes available.
Details
Details
- Reviewers
bartek varun - Commits
- rCOMMfadf7a78b5bf: [Tunnelbroker] Initial message persistence
nix develop # init localstack and run terraform comm-dev services start (cd services/terraform && ./run.sh) (cd services/tunnelbroker && cargo run &) (cd services/commtest && cargo test --test tunnelbroker_integration_test)
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Looks good to me but I'd also let @varun take a look
| services/commtest/tests/tunnelbroker_integration_test.rs | ||
|---|---|---|
| 55 ↗ | (On Diff #26432) | For future reviewers: This unwrap is refactored in the next diff 😉 |
| 96 ↗ | (On Diff #26432) | Just a general note: (for blob and backup we use the env variable for port number but this isn't clean and flexible enough) |
| services/commtest/tests/tunnelbroker_integration_test.rs | ||
|---|---|---|
| 55 ↗ | (On Diff #26432) | Yea, meant to comment. Thanks :) |
| 96 ↗ | (On Diff #26432) | Created https://linear.app/comm/issue/ENG-3940. But I agree, it would nice to create a paradigm for configuring this. |
Comment Actions
nvm misunderstood the persist_message fn. just a couple nits
| services/tunnelbroker/src/database.rs | ||
|---|---|---|
| 66 ↗ | (On Diff #26432) | nvm, i didn't realize that this is the sort key. would be helpful to label it similar to how you've labeled PARTITION_KEY two lines above |
| services/tunnelbroker/src/database.rs | ||
|---|---|---|
| 51 ↗ | (On Diff #26432) | no, created_at is a sort key. So there can be many. |
| 66 ↗ | (On Diff #26432) | I do have it as an alias: https://github.com/CommE2E/comm/blob/a79d72de974f45a44eefc5c3047f0477179841aa/services/tunnelbroker/src/constants.rs#L26 I chose to use the CREATED_AT since it more aligns with the business logic, and is easier for me to follow. I would also prefer to use DEVICE_ID, but we already established the usage of PARTITION_KEY in identity service. If you're adamant about preferring the current schema structure over field names, I can change it. |
| 73 ↗ | (On Diff #26432) | thanks |
| services/tunnelbroker/src/database.rs | ||
|---|---|---|
| 66 ↗ | (On Diff #26432) |
we've also established the usage of SORT_KEY in the identity service: i think it's fine to use 1) DEVICE_ID and CREATED_AT or 2) PARTITION_KEY and SORT_KEY, but as currently written, it's not clear that the table has a composite primary key |