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

Update to http 1.0 #59

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Update to http 1.0 #59

merged 1 commit into from
Nov 22, 2023

Conversation

davidpdrsn
Copy link
Contributor

No changes needed in the code but I updated the example to hyper 1.0 as well which required changes.

This is required for axum to support hyper 1.0.

@SergioBenitez
Copy link
Member

Please see the build failures.

@davidpdrsn
Copy link
Contributor Author

@SergioBenitez I believe the errors should be fixed now.

@davidpdrsn
Copy link
Contributor Author

@SergioBenitez do you have time to take a look at this? Its the only thing blocking the next major release of axum.

@SergioBenitez
Copy link
Member

Can you squash into a single commit?

@davidpdrsn
Copy link
Contributor Author

Done!

@SergioBenitez SergioBenitez merged commit d2fe51f into rwf2:master Nov 22, 2023
4 checks passed
@SergioBenitez
Copy link
Member

Ah, I see now that we're exposing types from http. This is rather unfortunate as it means we need to do a major version bump on multer. Unfortunately this means I'll need to do a bit more thorough of a review before committing to a 3.0.

Is this completely blocking you? Are you interfacing through the HeaderMap, and if so, would it be acceptable to convert the old map to the new map in the interim?

@davidpdrsn
Copy link
Contributor Author

Is this completely blocking you? Are you interfacing through the HeaderMap, and if so, would it be acceptable to convert the old map to the new map in the interim?

Not completely. The context is that we have a Field type which wraps multer::Field. Our Field has a headers method which returns &http::HeaderMap by simply delegating to the underlying multer::Field. So that would expose a HeaderMap from http 0.2. We can't convert the headers in our headers method because it must return a &HeaderMap that's borrowed from self.

I think our best option probably is to convert the headers eagerly when we construct our wrapper Field. That will hurt performance a bit but I'd be okay with that since it's just temporary. I wouldn't wanna remove axum's multipart handling (to then add it back in a point release) or publish a fork of multer.

@SergioBenitez
Copy link
Member

Got it, okay, that's great. I've moved this up on my priority list. I should be able to get to it fairly soon. Will keep you posted.

@davidpdrsn
Copy link
Contributor Author

Thanks!

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