- due to how interconnected this code is, it was hard to split it out into more diffs
- using the update_item API to add new users to the users table in DynamoDB was a mistake. we should be using put_item (hence why we now call the add_user_to_users_table fn, which uses the put_item API)
- since we know exactly how many messages we'll receive in the stream, we can avoid looping through messages on the server side
Details
Details
Called register_user and login_user from main.rs and confirmed that both workflows still work
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- identity (branched from master)
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| services/identity/src/database.rs | ||
|---|---|---|
| 398 ↗ | (On Diff #22506) | made this pub so we can use it in service.rs |
| 401 ↗ | (On Diff #22506) | Use our Error type here for better error handling in service.rs |
| services/identity/src/service.rs | ||
| 797–805 ↗ | (On Diff #22506) | Using .0 and .1 to access fields in a tuple is bad for readability, so I've replaced the tuples with structs |
| services/identity/src/service.rs | ||
|---|---|---|
| 697 ↗ | (On Diff #22506) | This file is already getting to be ~800 lines long, do you mind moving these modules to separate files? |
Comment Actions
Do we need to clone client and config so often? I'm assuming that config would be relatively static after creation. And some of the client clones seem to be unnecessary as they seem to fall out of scope from both scopes after their use.
| services/identity/src/service.rs | ||
|---|---|---|
| 797–805 ↗ | (On Diff #22506) | Yes, thank you. |
| services/identity/src/service/registration.rs | ||
| 42 | Don't think the clone is necessary anymore, we already take ownership of it, and don't use it again in the function. I think the best thing for pake_registration_start would be for it to take a reference to config, and just copy the key when needed. | |
Comment Actions
Good catch! I removed all the unnecessary clones and changed the helper functions to take a reference to Config
Comment Actions
Looks fine, probably a few ways to remove a lot of the boiler plate, but we can do that later.