Skip to content

Port to Tokio 0.2 #307

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

Closed
wants to merge 128 commits into from
Closed

Port to Tokio 0.2 #307

wants to merge 128 commits into from

Conversation

vorot93
Copy link
Contributor

@vorot93 vorot93 commented Aug 16, 2019

Quickly sew this together to get started with Tokio 0.2 runtime. Due to an additional trait bound in http-service I cut Zstd support. Also I removed App::run, although I don't think it will really be missed since async main has become commonplace.

Tested locally, appears to be working.

prasannavl and others added 30 commits May 15, 2019 06:51
fix cargo fmt

loosen up clippy to warnings on examples

snipe dev deps from core

eliminate some clippy warnings for examples

add publish false, and remove publish requirements

rename tide-examples to just examples
Fix example link in readme
Rename `serve` to `run`, add asynchronous `serve`
Check example in readme compiles during testing
Remove #[allow(unused_mut)]
Middleware-based compression and decompression
Improve curl command consistency
* Move core traits and types to tide-core

* Move cookies middleware to tide-cookies

* Remove unncessary dependencies and use relative paths in README
* Update travis config

 * Separate out individual build jobs for faster wall-clock testing

 * Fix clippy not actually denying warnings (excluded examples because
   these are currently failing and have non-trivial fixes)

 * Add build job that checks --no-default-features works

 * Add build job that checks for intra-doc-resolution failures (excluded
   tide because of bugs in re-exports with the intra-doc feature)

* Fix warnings

* Fix doc-link in tide-cookies
Signed-off-by: Yoshua Wuyts <[email protected]>
Signed-off-by: Yoshua Wuyts <[email protected]>
Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, but I think we can probably change out the git deps now that a new http-service has been published!

http-service = "0.3.1"
http-service-hyper = { version = "0.3.1", optional = true }
http-service = { git = "https://github.com/rustasync/http-service", branch = "tokio-0.2" }
http-service-hyper = { git = "https://github.com/rustasync/http-service", branch = "tokio-0.2", optional = true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed? I believe we already published updated versions right?

Copy link
Contributor Author

@vorot93 vorot93 Sep 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look closely, this actually depends on the branch. Specifically, this one:

http-rs/http-service#43

Besides, that branch depends upon git hyper anyway.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.3.1 was supposed to release earlier, but I only got it done today after seeing http-rs/http-service#45

Is there a reason we aren't using overrides for the Git dependencies in this branch? (Seems that would prevent issues with publishing, but correct me if I'm wrong.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure -- Git deps can't be published to crates.io either way.

@abonander
Copy link

What's the status on this? I wanted to see if my refactor on master of multipart-async was suitable for solving #89 yet but I built it against futures-*-preview 0.3.0-alpha.18 and I don't really want to downgrade (although I can if necessary).

@secretfader
Copy link

secretfader commented Oct 29, 2019

I removed the example that required runtime in dfbaf72 a few moments ago, which should make wrapping up this PR a little bit easier. Feel free to rebase against master at your discretion @vorot93.

If you need help, I'm now available to help push this over the finish line.

@vorot93
Copy link
Contributor Author

vorot93 commented Oct 31, 2019

@secretfader It looks like the whole repository has been ripped inside out. This PR should be redone from scratch now. Even then I'm not sure that things won't break again in the process.

@yoshuawuyts
Copy link
Member

@vorot93 hi! I'm currently working on rebasing past commits based on a single repo; this may take a few days. Thanks so much for the work you've done, and apologies for the inconvenience! -- I'm hoping we can take the work that was done here and land it again soonish!

@abonander
Copy link

@tyranron
Copy link

@abonander @yoshuawuyts why? Would you be so kind to explain the situation?

@abonander
Copy link

I'm only somewhat familiar with the circumstances. @yoshuawuyts can explain much better than I can.

@tyranron
Copy link

@abonander if so, why do you propose to close this PR referring only to a comment where author says he is banned? Is his work negligible or undesirable? From what I see in this PR it was sorta ongoing and OK, and then suddenly closed without any explanation 😕

@abonander
Copy link

abonander commented Nov 12, 2019

The tone of the author's comment in that thread implies they don't have access to the PR/repo to complete it and/or have no interest in finishing it. Giving any more detail means entering a discussion I have no desire to partake in. It was my comment they replied to so I felt obliged to acknowledge it.

@tyranron
Copy link

@abonander thanks for making this clear.

@yoshuawuyts
Copy link
Member

@tyranron the behavior of several individuals in withoutboats/romio#106 but also through other outlets (dms, forums, repos) has been disappointing and below the standards we expect for community participation. This has been the reason why they've lost the privilege of publicly engaging on repositories that are part of this organization.

Regarding upgrading to a modern runtime. That has already been implemented as part of #350, and I'll be kicking off the series of PRs required for a new release over the course of this week.

@tyranron
Copy link

@yoshuawuyts I've followed through the comments in withoutboats/romio#106 and in relevant Reddit posts, but clearly don't see any bad or disappointing behavior of @vorot93. I'm no one to judge your community standards, but the whole situation with banning contributors feels rather disappointing and even quite awkward.

Anyway, thanks for the clarification. And I'm finishing this offtopic.

@rubik
Copy link

rubik commented Oct 2, 2020

@yoshuawuyts I'm very late to the discussion, could you clarify if Tokio is/will be supported at all? I cannot find any mention of it in the README. I really like tide and I would like to use it for my projects, but there are some other libraries that only work with Tokio (Elasticsearch). My understanding is that mixing two runtimes is problematic, so it would be great if tide worked with Tokio as well.

@yoshuawuyts
Copy link
Member

@rubik async-std comes with a tokio02 feature flag that makes Tokio play nice with other runtimes. Using that should make it possible to use Tokio crates with Tide.

@rubik
Copy link

rubik commented Oct 2, 2020

@yoshuawuyts Ahh I did not know that. Thanks a lot! That's very promising!

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.