search

jonhoo / faktory-rs

Rust bindings for Faktory clients and workers
Apache License 2.0
205 stars 16 forks source link

Support remove queues, as well as pause all / resume all #59

Closed rustworthy closed 6 months ago

rustworthy commented 7 months ago

Addresses #56

This change is Reviewable

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 74.19355% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 66.7%. Comparing base (eac9545) to head (3b33790).

Additional details and impacted files | [Files](https://app.codecov.io/gh/jonhoo/faktory-rs/pull/59?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset) | Coverage Δ | | |---|---|---| | [src/proto/single/cmd.rs](https://app.codecov.io/gh/jonhoo/faktory-rs/pull/59?src=pr&el=tree&filepath=src%2Fproto%2Fsingle%2Fcmd.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset#diff-c3JjL3Byb3RvL3NpbmdsZS9jbWQucnM=) | `97.5% <100.0%> (+2.5%)` | :arrow_up: | | [src/proto/client/mod.rs](https://app.codecov.io/gh/jonhoo/faktory-rs/pull/59?src=pr&el=tree&filepath=src%2Fproto%2Fclient%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset#diff-c3JjL3Byb3RvL2NsaWVudC9tb2QucnM=) | `84.3% <73.3%> (+0.8%)` | :arrow_up: |
jonhoo commented 7 months ago

going to hold off on reviewing this until after #49

rustworthy commented 6 months ago

@jonhoo

I encountered some issues when trying to make Client hold a boxed dyn Connection . Our Reconnect trait mentions Self in the return value of it's only method, making the whole thing not object safe. So introducing a Client which is a struct with the following shape ...

type Connection = Box<FaktoryConnection>;
struct Client {
  connection: Connection,
  opts: ClientOptions,
 }

... did not work.

I might be missing something though..

Otherwise the PR is ready for review.

jonhoo commented 6 months ago

Would you mind splitting this PR into multiple, one for each logical change? It's a little hard to review the changes when multiple of them are bundled together in this way.

rustworthy commented 6 months ago

Would you mind splitting this PR into multiple, one for each logical change? It's a little hard to review the changes when multiple of them are bundled together in this way.

Sure

rustworthy commented 6 months ago

@jonhoo

Only queue control actions are left in this PR. Please have a look at it once again