Details
Run tests, make sure tests actually coding use cases by looking at Tunnelbroker logs.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| services/tunnelbroker/src/websockets/mod.rs | ||
|---|---|---|
| 199 ↗ | (On Diff #31966) | Should we unwrap here? Is it going to be handled gracefully? |
| services/tunnelbroker/src/websockets/session.rs | ||
| 184–188 ↗ | (On Diff #31966) | Why are we deleting a message if queue publish doesn't work? |
| 194–220 ↗ | (On Diff #31966) | If we are ignoring values of Err(_) (first match) and other message types (second match) we could just use let else. Separately I'm not sure where you are planning to put the "send message batch logic" but it might make sense to return MessageSentStatus here instead of SendConfirmation |
| 305 ↗ | (On Diff #31966) | Nit: imo it's better to specify the unit value so the reader doesn't have to think what value are we ignoring |
address review
| services/tunnelbroker/src/websockets/mod.rs | ||
|---|---|---|
| 199 ↗ | (On Diff #31966) | changed to break |
| services/tunnelbroker/src/websockets/session.rs | ||
| 184–188 ↗ | (On Diff #31966) | The publish result is still Ok when recipient is not connected, and messages are persisted in the correct order. This will fail when things like connection with the RMQ broker is broken. I think it's okay then to inform the client that something went wrong, but I don't feel strongly, we can discuss this if you have a different point of view. |
| 194–220 ↗ | (On Diff #31966) |
right, thanks
makes sense |