Skip to content
This repository has been archived by the owner on Feb 27, 2024. It is now read-only.

Upgrade the project to Actix 4.0.0 #13

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

stevefan1999-personal
Copy link

@stevefan1999-personal stevefan1999-personal commented Jan 12, 2021

This is not yet finalized, as there is some substantial change in the source code.

More specifically, there happen to have Request type param removed in several traits, and that MailboxError does not implement ResponseError anymore. I substituted ServiceRequest into the type param and it should work as intended.

I workaround the latter by adding an extra function call ErrorInternalServerError using map_err to wrap the error up. It's a convenient workaround but it's not very effective. I checked the diff from 3.x to 4.x and see that from MailboxError to ResponseError is transformed to an ISE, this is how I come up with this idea.

Yet, copy pasting map_err(ErrorInternalServerError) isn't very fun and tedious, but since this is an upstream Actix problem, why don't we just go blind first for a few moments.

Until the review passed on the ISE part and all the crate has graduated from beta then, let's keep it a draft.

@tomcraven tomcraven mentioned this pull request Jan 16, 2021
@vicky5124
Copy link

vicky5124 commented Apr 3, 2021

Is this going forward? Actix 4 is long out of beta (only actix-rt, upz)

@Kavan72
Copy link

Kavan72 commented Sep 17, 2021

any update of this one ?

@Tristaan
Copy link

The user making this has dissapeared from the face of the world. So now somebody else has to do it. I'll check it out.

@stevefan1999-personal stevefan1999-personal marked this pull request as ready for review November 11, 2021 05:19
@poisson-fish
Copy link

The user making this has dissapeared from the face of the world. So now somebody else has to do it. I'll check it out.

See my pull request here for working actix4 code

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants