Details
Run tests to make sure existing logic is the same. For farcaster_tokens I connected my DataGrip to my localstack, added some logging in tests and for each tests I was checking if values are showing up or are deleted.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| services/identity/src/database/farcaster.rs | ||
|---|---|---|
| 71–75 ↗ | (On Diff #49561) | In theory, I should've updated this, too, but this will be a complicated operation where I don't think this is needed at all. I tried to implement a test for it as part D15206, and that case (need to remove DCs token) isn't reachable. Updating ID for a user who already has it is gated here, and it isn't possible to add a token when there is no ID here, I think we should just remove REMOVE {1} cc. (@tomek) |
| services/identity/src/database/farcaster.rs | ||
|---|---|---|
| 71–75 ↗ | (On Diff #49561) | Yeah, I think it makes sense. Connecting a Farcaster account requires disconnecting first, which removes the token, so there are no valid scenarios where the token is set in this function. |
| services/identity/src/database.rs | ||
|---|---|---|
| 318–323 ↗ | (On Diff #49561) | Let's avoid cloning when it's none |
| services/identity/src/database/farcaster.rs | ||
| 139 ↗ | (On Diff #49561) | Separately, in the future this MissingItem could be improved to provide more details to LinkFarcasterDCsAccount RPC handler, but for now such log should be sufficient to investigate what went wrong |
| 238 ↗ | (On Diff #49561) | You don't have to clone here |
| 251–254 ↗ | (On Diff #49561) | A better way when transact items are fixed |
| 255–263 ↗ | (On Diff #49561) | Do you think we should add exponential backoff to this transaction? Is there any chance Tunnelbroker tries to write it at the same time? |
| 296 ↗ | (On Diff #49561) | DynamoDB errors better implement Debug trait than Display |
- avoid clonning None
- delete REMOVE {1} clause
- improve logging
- add exponential backoff