This method, as explained in the diff title, lets us store user tokens in DynamoDB. Note that we convert the DateTime type to an RFC 3339 string. This seemed like the cleanest available option.
Depends on D3961
Details
Details
successfully put a token in DDB
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| services/identity/src/database.rs | ||
|---|---|---|
| 242–243 ↗ | (On Diff #12387) | I know this comes from the db (right?) but shouldn't we add unsupported here? |
Comment Actions
I'm not sure why this is in an "accepted" state. I hope it cannot be landed... After I accept, rust diffs should not be accepted, I know rust far too little to be decisive. Adding Jim as blocking.
Comment Actions
When any reviewer accepted a diff and there are no blocking reviewers the state is accepted. If we want more people to accept before landing, we need to set them as blocking - just as you did.
Comment Actions
Just a couple ways to make things more concise and/or readable. Feel free to ignore if you don't like it.
| services/identity/src/database.rs | ||
|---|---|---|
| 234 ↗ | (On Diff #12387) | nit: I'd write this as let created_string = token.created
.ok_or_else(|| {
error!(...);
Error::MissingAttribute
})?; |
| 249 ↗ | (On Diff #12387) | nit: I'd write this as let valid_bool = token.valid
.ok_or_else(|| {
error!(...);
Error::MissingAttribute
})?; |
| 308 ↗ | (On Diff #12387) | This can just be map_err(Error::RusotoPut) or even map_err(Into::into) |
| services/identity/src/database.rs | ||
|---|---|---|
| 234 ↗ | (On Diff #12387) | I'd have to write another line to convert the DateTime<Utc> to a String, right? |
| 242–243 ↗ | (On Diff #12387) | this doesn't come from the db. auth_type is an enum in the Token struct with only two variants, so there's nothing else to handle in this match statement |
| 249 ↗ | (On Diff #12387) | makes sense |
| 308 ↗ | (On Diff #12387) | makes sense |
Comment Actions
@varun you may need to re-request review to get @jimpo's attention so it shows up on his queue. If that doesn't work feel free to ping him about your question