Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bumped dependency versions #63

Merged
merged 5 commits into from
Jun 24, 2021
Merged

Conversation

pranav-bhatt
Copy link
Contributor

The main motivation behind this was that thetokio version being used in this crate was still 0.2 and this was causing major issues in my project. Refactored it to use tokio 1.7*, and also ran cargo fmt (Do tell me if this might cause issues).

I also wanted to ask if this warrants a version bump since the change in tokio versions might cause issues for others.

Signed-off-by: pranav <adpranavb2000@gmail.com>
Signed-off-by: pranav <adpranavb2000@gmail.com>
@pranav-bhatt
Copy link
Contributor Author

pranav-bhatt commented Jun 16, 2021

Some of the tests failed, not sure why :/ :

failures:
[00:02:58] 
[00:02:58] ---- observer::test::test_observe stdout ----
[00:02:58] thread 'main' panicked at 'assertion failed: `(left == right)`
[00:02:58]   left: `[79, 75]`,
[00:02:58]  right: `[100, 97, 116, 97, 49]`', src\observer.rs:480:26
[00:02:58] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[00:02:58] 
[00:02:58] ---- observer::test::test_unobserve stdout ----
[00:02:58] thread 'main' panicked at 'assertion failed: `(left == right)`
[00:02:58]   left: `[79, 75]`,
[00:02:58]  right: `[100, 97, 116, 97, 49]`', src\observer.rs:525:17
[00:02:58] 
[00:02:58] ---- server::test::multicast_server_all_coap stdout ----
[00:02:58] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 10060, kind: TimedOut, message: "A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond." }', src\server.rs:613:44
[00:02:58] 
[00:02:58] ---- server::test::test_echo_server_no_token stdout ----
[00:02:58] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 10060, kind: TimedOut, message: "A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond." }', src\server.rs:494:44
[00:02:58] 
[00:02:58] ---- server::test::test_update_resource stdout ----
[00:02:58] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 10060, kind: TimedOut, message: "A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond." }', src\server.rs:540:26

EDIT:
I apologise for not being clear enough. I not sure why the payload is being modified

@Covertness
Copy link
Owner

It seems the payload was changed

Signed-off-by: pranav <adpranavb2000@gmail.com>
@pranav-bhatt
Copy link
Contributor Author

pranav-bhatt commented Jun 18, 2021

It seems the payload was changed

@Covertness So it seems the response that is being received is the one from the response_handler, i.e, b"OK". I'm not even sure how in the original repo this test case was passing. Shouldn't the client.receive() be getting "OK" as the response always in the original code?

Would you be able to take a look? Hopefully shouldn't take too long :)

@Covertness
Copy link
Owner

The problem happened on this line. The socket always produce Poll::Ready result after the first frame reached. It maybe caused by the new version of tokio_util::udp::UdpFramed

@pranav-bhatt
Copy link
Contributor Author

pranav-bhatt commented Jun 19, 2021

Are you a part of an IRC or Slack server so I could ping you there?

The problem happened on this line. The socket always produce Poll::Ready result after the first frame reached. It maybe caused by the new version of tokio_util::udp::UdpFramed

I'll try downgrading this and check.

@pranav-bhatt
Copy link
Contributor Author

I'll try downgrading this and check.

Downgrading to a version below 0.6 unfortunately breaks stuff. Guess we'll have to figure out another way.

@Covertness
Copy link
Owner

The new version has its own Stream. I think it will be compatible with the socket of new version.

@pranav-bhatt
Copy link
Contributor Author

pranav-bhatt commented Jun 20, 2021

The new version has its own Stream. I think it will be compatible with the socket of new version.

Do correct me if I'm wrong, but from what I can see, the Stream trait is just a reexport of the futures::stream::Stream trait, so I don't think this is the issue. Quoting tokio_stream::StreamExt:

Be aware that the Stream trait in Tokio is a re-export of the trait found in the futures crate, however both Tokio and futures provide separate StreamExt utility traits, and some utilities are only available on one of these traits. Click here to see the other StreamExt trait in the futures crate.

If you need utilities from both StreamExt traits, you should prefer to import one of them, and use the other through the fully qualified call syntax.

The necessary functions like poll_next_unpin() are imported from futures::StreamExt, so from what I can see, tokio_stream::StreamExt is also not necessary.

I've also started a discussion here in the tokio repo :)

@Covertness
Copy link
Owner

Okay. Waiting for the reply from the tokio team.

@pranav-bhatt
Copy link
Contributor Author

pranav-bhatt commented Jun 21, 2021

Okay. Waiting for the reply from the tokio team.

@Covertness I had a couple of other issues, and I was hoping that we could communicate real-time over a chat service. Could I possibly have your slack or twitter handle (or any other chat service that suits you)? My email ID is adpranavb2000@gmail.com

Signed-off-by: pranav <adpranavb2000@gmail.com>
@pranav-bhatt pranav-bhatt marked this pull request as ready for review June 23, 2021 09:39
@pranav-bhatt
Copy link
Contributor Author

Can we have a new release? v1.0 wink wink

@Covertness
Copy link
Owner

Sorry for not getting back to you sooner. So busy recently. You can touch me by email if you want communicate with me. My email is wuyingfengsui@gmail.com.
Thank you for your pr. I will release it.

@Covertness Covertness merged commit 94fda8e into Covertness:master Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants