feat: port unregister command to Rust #1224
Conversation
bbangert
force-pushed
the
feat/issue-1205
branch
2 times, most recently
from
c27c9fd to
6bbb545
Compare
Codecov Report
@@ Coverage Diff @@
## master #1224 +/- ##
======================================
Coverage 100% 100%
======================================
Files 60 60
Lines 10198 10198
======================================
Hits 10198 10198
Continue to review full report at Codecov.
|
bbangert
force-pushed
the
feat/issue-1205
branch
2 times, most recently
from
40c842f to
4e24c8d
Compare
autopush_rs/src/util/ddb_helpers.rs
Outdated
| .and_then(move |_| -> MyFuture<_> { Box::new(future::ok(true)) }) | ||
| .or_else(move |_| -> MyFuture<_> { Box::new(future::ok(false)) }); | ||
| if code != 200 { | ||
| info!("Unregister"; |
There was a problem hiding this comment.
did we want the info! for register then? You said we didn't need it in the other PR
There was a problem hiding this comment.
I don't think we care about successful registers (they all are), but unsubscribes are different cause they can happen for non-user-driven reasons like quota expiration, etc. The Python code always logged the unregisters, which like the registers is a bit pointless. But we definitely want to know about the codes involved in non-user-driven unregisters. Though, I think we want a tagged metric now that you mention it, not a log statement.
autopush_rs/src/client.rs
Outdated
| @@ -990,22 +994,26 @@ where | |||
| ) -> Poll<AfterAwaitUnregister<T>, Error> { | |||
| debug!("State: AwaitUnRegister"); | |||
| let msg = match try_ready!(await_unregister.response.poll()) { | |||
| call::UnRegisterResponse::Success { success } => { | |||
| true => { | |||
There was a problem hiding this comment.
since this is a bool now, I think you'd save a few lines of code making this an if/else instead of a match. especially since both blocks return ServerMessage::Unregister
The register command PR should be merged and this rebased on top of master before this is reviewed.
Closes #1205.