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

Upgrade http to 0.2 #199

Closed
wants to merge 1 commit into from
Closed

Upgrade http to 0.2 #199

wants to merge 1 commit into from

Conversation

LinusU
Copy link

@LinusU LinusU commented Jan 19, 2020

Description of changes: Bumps the http crate to version 0.2

closes #196

change log: https://github.com/hyperium/http/releases/tag/v0.2.0

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

@LinusU
Copy link
Author

LinusU commented Jan 26, 2020

Hmmm, I think that the build failure is that http 0.2 introduces a dependency on bytes which doesn't compile under Rust 1.31.

Would you be open to upgrading the minimum required Rust version? ☺️

@davidbarsky
Copy link
Contributor

@LinusU Yeah, bumping the minimum supported Rust version would help. That being said, I am a bit worried about introducing http 0.2 without also bumping Hyper to 0.13, as there will be duplicate instances of http and the compiler errors you can get with duplicate crates are really confusing.


For what it's worth, we're also doing bump of http in #111, and @sapessi and I have been working on that PR this weekend to get it complete.

@LinusU
Copy link
Author

LinusU commented Jan 27, 2020

as there will be duplicate instances of http and the compiler errors you can get with duplicate crates are really confusing.

Hehe, yeah, in fact getting those error messages and spending a lot of time trying to figure out why is why I bumped these versions 😁

Getting #111 in is probably a better idea as you say 👍

My use case is to slightly modify the hyper http server to feed it requests from Lambda, so that you can take any already existing http server that's based on hyper (e.g. a warp app) and deploy it to Lambda with just one command (scandium)


Should we just close this one out in favour of #111?

@davidbarsky
Copy link
Contributor

My use case is to slightly modify the hyper http server to feed it requests from Lambda, so that you can take any already existing http server that's based on hyper (e.g. a warp app) and deploy it to Lambda with just one command (scandium)

Ah yeah. One of the reasons #111 took a while was:

  1. Procrastination on my part.
  2. Trying to support the use-case you described in an elegant way without specialization or negative bounds. I don't think it can be done elegantly and the less-elegant way is perfectly fine and it would involve usage of tower::Service. I believe Warp now supports converting its filters to a tower::Service now.

Should we just close this one out in favour of #111?

Perhaps—feel free to pull down my branch and try it out for your needs!

@LinusU
Copy link
Author

LinusU commented Apr 1, 2020

@davidbarsky rebased this on master after #111 was closed 🎉

It seems like lambda-http is no longer included in the workspace, was this intentional? I'd be happy to submit a PR otherwise...

edit: actually, it seems like there is a bit of work required in lambda-http, it currently tries to use a number of things from lambda-runtime, but that package was removed in #111

The readme also mentions lambda-runtime and probably needs updating 🤔

@davidbarsky
Copy link
Contributor

Thanks for the rebase!

It seems like lambda-http is no longer included in the workspace, was this intentional? I'd be happy to submit a PR otherwise...

It was intentional, as it'll require a decent amount of fixing up to work with the new rutime semantics and for it to support the new Lambda invocation format introduced with API Gateway's HTTP APIs. I'll accept a PR, but I also know its a non-trivial amount of work to be done correctly.

The readme also mentions lambda-runtime and probably needs updating 🤔

Yup! 😅

@softprops
Copy link
Contributor

This can be closed now. An upgrade with the http crate came with the last update to the lambda http module

@LinusU LinusU closed this May 27, 2020
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.

3 participants