AtherEnergy / rumqtt

Pure rust mqtt cilent
The Unlicense
202 stars 72 forks source link

Fix MqttOptions setters #144

Closed manifest closed 5 years ago

manifest commented 5 years ago

The following code fails because the current implementation of setters require implementation of Copy trait.

let mut opts = rumqtt::MqttOptions::new(client_id, host, port.as_u16());
if let Some(value) = config.clean_session {
    opts.set_clean_session(value);
};

Ok(opts)
error[E0382]: use of moved value: `opts`
   --> /Users/manifest/projects/netology-group/svc-agent-rs/src/mqtt.rs:107:12
    |
103 |             opts.set_clean_session(value);
    |             ---- value moved here
...
107 |         Ok(opts)
    |            ^^^^ value used here after move
    |
    = note: move occurs because `opts` has type `rumqtt::mqttoptions::MqttOptions`, which does not implement the `Copy` trait

error: aborting due to previous error

For more information about this error, try `rustc --explain E0382`.
error: Could not compile `svc-agent`.

To learn more, run the command again with --verbose.
manifest commented 5 years ago

I've also changed the type of keep_alive option to u64 since it is expected by Duration constructor and to make it symmetrical to other options using time intervals in seconds as reconnect.

tekjar commented 5 years ago

Thank you :)

manifest commented 5 years ago

@tekjar Could we bump a new crate version? We need this change in another crate, so we can't use branch or just depend on the commit there.

tekjar commented 5 years ago

Ahh, I'm not sure if there are any breaking changes in the master branch since the last release. Can you let me know if there are any? I'll do minor increment if not

manifest commented 5 years ago

Can you let me know if there are any?

There is none of breaking changes as far as I can see. We can go with update of the minor version.

tekjar commented 5 years ago

Looks like this is breaking a bunch of tests and examples. I might have missed the CI here. We have to revert this

tekjar commented 5 years ago

I'll revert this now. I need some consensus on builder patterns. Can you use this for now?

let mut opts = rumqtt::MqttOptions::new(client_id, host, port.as_u16());
let opts = if let Some(value) = config.clean_session {
    opts.set_clean_session(value)
} else {
    opts
}

Ok(opts)