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

Usage with axum difficult #9

Closed
MTRNord opened this issue Nov 21, 2022 · 5 comments
Closed

Usage with axum difficult #9

MTRNord opened this issue Nov 21, 2022 · 5 comments

Comments

@MTRNord
Copy link
Contributor

MTRNord commented Nov 21, 2022

Hi I am trying to use this with axum instead of axtix-web.

However it seems that into_apub from ApubObject causes:

note: future is not `Send` as it awaits another future which is not `Send`
   --> src/instance.rs:133:18
    |
133 |     let person = user.into_apub(&data).await.unwrap();
    |                  ^^^^^^^^^^^^^^^^^^^^^ await occurs here on type `std::pin::Pin<std::boxed::Box<dyn std::future::Future<Output = std::result::Result<objects::person::Person, error::Error>>>>`, which is not `Send`

I assume this is because the function takes an reference to the Arc instead of taking the arc itself which somewhat breaks the Arc's purpose.

Is there any way this could be made axum compatible?

My (broken) Code for this is available here (it also wont compile due to some other reasons as I am in between adding basic functions but got stuck on this issue): https://gitlab.com/MTRNord/pixelrust/-/blob/dc50d4bd729eecc23d972d6d3d92426edb280911/src/instance.rs#L132

@MTRNord
Copy link
Contributor Author

MTRNord commented Nov 21, 2022

A possible easy fix is probably to just not force it to be a reference and let the implementer decide if this should be a reference or not. That way this would offer the best of both worlds. (Basically moving the reference to the type definition when implementing instead of forcing it always)

@Nutomic
Copy link
Member

Nutomic commented Nov 22, 2022

I looked at your axum-error branch but its really hard to figure out whats going on, because there are so many changes (all in one huge commit), and lots of unrelated compile errors. You also seem to have merged the library code and example code which is a bad idea.

If you want to add axum support to this crate, you should instead use feature flags, with one feature actix and one axum. Then you can also rename the federation example to federation-actix, and add federation-axum.

I suggest you work on this in a pull request to this repo, and only commit small changes each time. That way it will be much easier to follow along and help out if you get stuck somewhere.

@MTRNord
Copy link
Contributor Author

MTRNord commented Nov 22, 2022

I looked at your axum-error branch but its really hard to figure out whats going on, because there are so many changes (all in one huge commit), and lots of unrelated compile errors. You also seem to have merged the library code and example code which is a bad idea.

If you want to add axum support to this crate, you should instead use feature flags, with one feature actix and one axum. Then you can also rename the federation example to federation-actix, and add federation-axum.

I suggest you work on this in a pull request to this repo, and only commit small changes each time. That way it will be much easier to follow along and help out if you get stuck somewhere.

Ah sorry yeah shouldn't have this written late at night yesterday 😅

I will clear the git up later today or tomorrow and come back but the short version is that I basically took the example and tried to use axum instead of actix. Therefor I had to port the middleware to axum as that also is (logically) actix Web specific.

I can probably work on a pr for axum as well. :) will have a look later today.

@MTRNord
Copy link
Contributor Author

MTRNord commented Nov 24, 2022

Just wanted to note that I started work on this now :) Its not pushed yet but I will keep you updated/open the PR when its ready

@oknozor
Copy link
Contributor

oknozor commented Jan 16, 2023

I think this should be closed, the only remaining issue with axum is that we still need to run on the actix runtime but this should be fixed by making the background-jobs crate compatible with tokio. Or maybe switch to another crate.

@MTRNord here is my project if you need to take a look at a working example: https://github.com/oknozor/gill

@Nutomic Nutomic closed this as completed Feb 25, 2023
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

No branches or pull requests

3 participants