feat: port check storage command into Rust #1202
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1202 +/- ##
=======================================
Coverage 100% 100%
=======================================
Files 60 60
Lines 10194 10344 +150
=======================================
+ Hits 10194 10344 +150
Continue to review full report at Codecov.
|
| if !RE.is_match(key) { | ||
| return Err("Invalid chidmessageid".into()).into(); | ||
| } | ||
| if key.starts_with("01:") { |
There was a problem hiding this comment.
part of me wonders if this should be a match...
There was a problem hiding this comment.
I don't think strings can be destructured to partial match right now.
There was a problem hiding this comment.
it's easily a match once the duplicated split(":") line is done beforehand
Co-authored by: Phil Jenvey <[email protected]> Some components of CheckStorage are left in Python as they're used by the delete tests. Closes #1189
| if !RE.is_match(key) { | ||
| return Err("Invalid chidmessageid".into()).into(); | ||
| } | ||
| if key.starts_with("01:") { |
There was a problem hiding this comment.
it's easily a match once the duplicated split(":") line is done beforehand
| encoding: Option<String>, | ||
| } | ||
|
|
||
| fn insert_to_map(map: &mut HashMap<String, String>, val: Option<String>, name: &str) { |
There was a problem hiding this comment.
nit: order of val, name vs name, val mildly awkward
| let response = response.and_then(|data| { | ||
| let notifs: Option<Vec<Notification>> = data.items.and_then(|items| { | ||
| debug!("Got response of: {:?}", items); | ||
| // TODO: Capture translation errors and report them as we shouldn't have corrupt data |
There was a problem hiding this comment.
we should handle this now w/ all the serde_dynamodb errors we added
There was a problem hiding this comment.
Yea, I was saving this for after we go to Sentry 0.3 so we can easily log them out to Sentry.
| } | ||
| Ok(FetchMessageResponse { | ||
| timestamp, | ||
| messages: notifs, |
There was a problem hiding this comment.
I forgot to go back and make this None when notifs.len() was 0 to match the python code exactly. I think it's better to make FetchMessageResponse's messages a raw Vec vs Option
There was a problem hiding this comment.
We'll have to change how the caller and responder work to do that as well, which is a bit more of a change than just porting the Python side.
refactor: some cleanup for #1202
Co-authored by: Phil Jenvey [email protected]
Some components of CheckStorage are left in Python as they're used
by the delete tests.
Closes #1189