Where communities thrive

Join over 1.5M+ people
Join over 100K+ communities
Free without limits
Create your own community

Explore more communities

gotham-rs/gotham

A flexible web framework that promotes stability, safety, security and speed.

People

Repo info

Activity

Oct 20 15:55

nyarly commented #472
Oct 20 15:30

msrd0 commented #483
Oct 19 16:13

nyarly labeled #483
Oct 19 16:13

nyarly opened #483
Oct 19 08:00

dependabot-preview[bot] labeled #482
Oct 19 08:00

dependabot-preview[bot] opened #482
Oct 19 08:00

dependabot-preview[bot] on cargo

Update tokio-rustls requirement… (compare)
Oct 16 07:24

dependabot-preview[bot] labeled #481
Oct 16 07:24

dependabot-preview[bot] opened #481
Oct 16 07:24

dependabot-preview[bot] on cargo

Update tokio requirement from 0… (compare)
Oct 14 14:41

msrd0 on cargo

(compare)
Oct 14 14:41

msrd0 on master

Update pin-project requirement … (compare)
Oct 14 14:41

msrd0 closed #480
Oct 14 07:54

codecov[bot] commented #480
Oct 14 07:34

codecov[bot] commented #480
Oct 14 07:14

dependabot-preview[bot] labeled #480
Oct 14 07:14

dependabot-preview[bot] opened #480
Oct 14 07:14

dependabot-preview[bot] on cargo

Update pin-project requirement … (compare)
Oct 13 12:46

msrd0 on cargo

(compare)
Oct 13 12:46

msrd0 on master

Update nix requirement from 0.1… (compare)

Judson Lester

@nyarly

And I can report that the docs there could be improved. HandlerMarkeris an opaque type, and I think the explanation of the tradeoff there could back up a step. I'll put in a docs PR shortly.

Denis Lisov

@tanriol

@nyarly I need to think about this, but I think it would not. The problem is that we cannot write the bound as for<'s> Fn(&'s mut State) -> impl Future<...> + 's because impl Future cannot be used in that position.

Judson Lester

@nyarly

I'm also thinking, as I work with this, that it would be ideal not to have both methods :/. Maybe a future version has async_to and we deprecate to_async*

Denis Lisov

@tanriol

It could be just a .to(...) that works with everything... if it worked.

Judson Lester

@nyarly

I think we'd have async fn wrapper(&'s mut State) -> Resultand then take the handler and pass it into that?

Denis Lisov

@tanriol

I don't really understand what we'd win in this case...

Judson Lester

@nyarly

If we could have to_async_borrowing that worked with closures and functions, that simplifies that use case an awful lot.

Denis Lisov

@tanriol

...honestly, I'd prefer to make .to(|name: SomethingFromState, name2: SomethingElseFromState, ...| -> Result<...> async { ... }) work...

Judson Lester

@nyarly

Looking at it, I'm not sure that could happen on the Gotham side though.

Oh, certainly.

Hm. I'm not sure about the magical unpacking that's implied there.... Well, based on the hlist behavior of the borrowbag, though.

Naming that type seems rough though.

Denis Lisov

@tanriol

I'm not sure whether it could be made to work with multiple function arguments, but at some point I had it working with a tuple of these...

msrd0

@msrd0_gitlab

I guess that could actually work if we had an impl Handler<A, B, ...> for Fn(A, B, ...) -> Result<...> where A: BorrowFromState, ...

Judson Lester

@nyarly

That's not terrible. It winds up being a multi-impl trait, right? impl X for (FromState); impl X for (FromState, FromState) ...

I'd be okay with a private macro to do defs for 0..32 different FromStates

Denis Lisov

@tanriol

Not ideal either - matching types to names looked a bit weird in the tuple case if there can be a number of these in the tuple...

Judson Lester

@nyarly

Howso?

Denis Lisov

@tanriol

Also, there's a drawback: customizing the response if something is missing is either impossible or requires some more wrappers.

msrd0

@msrd0_gitlab

I guess we could also make name: Option<SomethingFromState> work

is there any benefit on using tuples over several method arguments?

Denis Lisov

@tanriol

Several method arguments is a bit more difficult to implement. Formally speaking, impl<A> Trait for Fn(A) and impl<A, B> Trait for Fn(A, B) do overlap because something can have both implementations. Requires a workaround I did not know at that point.

Judson Lester

@nyarly

I'd consider it a necessary evil, if it is necessary. But part of having a "define 32 impls" macro would be having the implementation of tuples from the state, and then handlers with variable arguments that get the tuple from the state and then pass to the handler.

msrd0

@msrd0_gitlab

oh I see, that's annoying

Judson Lester

@nyarly

Option<FromState> also might get unwieldy. You go from N implementations to 2^N.

We could say "Options follow required" and keep it to N^2.

msrd0

@msrd0_gitlab

one way to circumvent that would be to intruce a handler!(|name: A, name2: B| unimplemented!()) macro that expands to |foo: (A, B)| { let (name, name2) = foo; unimplemented!() }

Judson Lester

@nyarly

@tanriol I'm not sure I follow. How can a type be Fn(A) and Fn(A, B)?

msrd0

@msrd0_gitlab

it cannot but rustc thinks it's possible

I forgot how annoying traits can be sometimes

Judson Lester

@nyarly

Is there a bug for that?

Denis Lisov

@tanriol

It can in Nightly where it's possible to implement the Fn family traits manually.

Judson Lester

@nyarly

Curse you, "always stable"! :)

msrd0

@msrd0_gitlab

I don't think it'll be possible unless rust found a way for trait impls to exclude each other

Judson Lester

@nyarly

Wait, what if it were impl<A> Trait1 for Fn(A) and impl<A,B> Trait2 for Fn(A,B) etc?

Denis Lisov

@tanriol

You've just moved the problem to an overlap between Trait1 and Trait2 :-)

Judson Lester

@nyarly

Ah! Right.

Denis Lisov

@tanriol

I think I'm going to prototype a to_any or something like that and check what exactly explodes :-)

msrd0

@msrd0_gitlab

sounds good

Judson Lester

@nyarly

Sounds awesome, actually

Denis Lisov

@tanriol

By the way, does anyone know why we need the StateData derive?

Is it needed for some technical reason or is this just "being able to put anything Any + Send there can be confusing, let's restrict it"?

msrd0

@msrd0_gitlab

the docs call it a marker trait and its derive doesn't do anything so I assume it's the later

Denis Lisov

@tanriol

StaticResponseExtender also looks confusing... I have no idea how it's supposed to be used :-)

msrd0

@msrd0_gitlab

that is very confusing, I agree

what it does is configure a blanket response in case the whatever-it-is-implemented-on operation failed

so if you are matching /search?query=... but the client didn't send the query param, the extender is called (and, if using the derive, configures the response as a 400 Bad Request)

Denis Lisov

@tanriol

If we were doing breaking changes for the sake of user sanity, I'd suggest renaming it to actually mention failure :-)

(but I'll try to think whether there's a better API for that)

msrd0

@msrd0_gitlab

it's a terribly confusing way of error handling, and I only understood what it did after removing it from gotham and looking at the compile errors