search

ironthree / dxr

Declarative XML-RPC in Rust
Apache License 2.0
17 stars 8 forks source link

Custom Client Transport #2

Closed blaenk closed 1 year ago

blaenk commented 1 year ago

Hi! Nice crate you have here among the dearth of abandoned xmlrpc crates.

I'm curious if you'd be open to making it possible to swap out reqwest for something custom in the client, probably by abstracting it out to a trait or something. If you're curious, I'm interested in using this with scgi over both raw tcp and unix domain sockets.

I'm happy to supply that implementation on my end, I'm mainly interested in having a way to supply it. Maybe it already is possible and I simply misunderstood the code I read.

decathorpe commented 1 year ago

Yeah, supporting other ways of connecting a client to a server is definitely something I want to do. It just hasn't happened yet, because for now I've only been using reqwest.

The plan was to make it possible to use different client libraries, similar to the way axum server support is already factored out from general-purpose XML-RPC server functionality, probably by defining a "Client" trait that can be implemented by different backends.

Which libraries would you use for implementing transport over raw TCP / Unix domain sockets? Maybe hyperlocal?

We might need to discuss what should be in the new "Client" trait to make sure it can work for reqwest (HTTP) and other transport mechanisms / libraries, but the important bits would probably be:

Is there something I missed? Or do some parts of that not work for the transport mechanisms you want?

blaenk commented 1 year ago

Hey thank you for responding, sorry for the delay.

The libraries I would use are just tokio and it's TcpStream or UnixStream. Thanks for telling me about hyperlocal because I didn't know that it existed, but I need to use SCGI to communicate with the application and I'm not sure if that supports it. I looked around for existing scgi crates but ended up rolling my own.

Your assessment sounds reasonable to me. I personally don't need to set headers aside from the ones already set by the underlying SCGI transport (SCGI and CONTENT_LENGTH), so I agree it is optional.

I'm not sure (not doubting it either, though) whether client initialization has to be part of the trait, maybe the implementing type can have its own initializer like your typical new() and the trait only concerns itself with sending a request, for example, but I defer to you on it, I'm not arguing that point myself.

As for sending the request, I imagine you're saying POST as an example of what it would be in HTTP since in SCGI there is no distinction of that kind. Not being pedantic, simply sharing the details. In my case the endpoint would just be e.g. the SocketAddr or the unix domain socket path. The body could probably be Bytes/Vec<u8>/&[u8] for maximum flexibility vs imposing String and its encoding requirements (unless XML-RPC already dictates it must be UTF-8, then it probably doesn't matter). My own implementation uses Bytes/BytesMut.

By the way, no rush or pressure on any of this of course, I managed to get something working for my application, it's more that I stumbled upon your project and thought it looked great compared to the alternatives, which caused me to roll my own. I just wanted to share that it could be more widely applicable (well, by +1) if the transport/client were flexible/customizable, since I can't use it in its current state for what I'm doing.

decathorpe commented 1 year ago

Hey thank you for responding, sorry for the delay.

No problem!

The libraries I would use are just tokio and it's TcpStream or UnixStream. Thanks for telling me about hyperlocal because I didn't know that it existed, but I need to use SCGI to communicate with the application and I'm not sure if that supports it. I looked around for existing scgi crates but ended up rolling my own.

That makes sense.

Your assessment sounds reasonable to me. I personally don't need to set headers aside from the ones already set by the underlying SCGI transport (SCGI and CONTENT_LENGTH), so I agree it is optional.

I didn't know about SCGI - is looks similar to HTTP?

I'm not sure (not doubting it either, though) whether client initialization has to be part of the trait, maybe the implementing type can have its own initializer like your typical new() and the trait only concerns itself with sending a request, for example, but I defer to you on it, I'm not arguing that point myself.

You're right, the initialization can be specific to each type that implements the trait, and doesn't need to be part of the trait. Some scenarios will probably be better for using a builder pattern, anyway.

As for sending the request, I imagine you're saying POST as an example of what it would be in HTTP since in SCGI there is no distinction of that kind. Not being pedantic, simply sharing the details. In my case the endpoint would just be e.g. the SocketAddr or the unix domain socket path.

Right. The XML-RPC spec specifies that it's transport protocol is HTTP, and that the only accepted HTTP method for XML-RPC requests is POST. But that's an implementation detail of the HTTP client, and would not apply to other client implementations, I guess.

The body could probably be Bytes/Vec<u8>/&[u8] for maximum flexibility vs imposing String and its encoding requirements (unless XML-RPC already dictates it must be UTF-8, then it probably doesn't matter). My own implementation uses Bytes/BytesMut.

True, the current implementation only supports String as payload (i.e. valid UTF-8), because the XML de/serialization only works with UTF-8. Supporting arbitrary encodings or bytes is not going to be easy ... unless you'd implement custom XML de/serialization for every client. The client based on reqwest only supports UTF-8 right now, as that's what's supported by the serde feature of quick-xml.

By the way, no rush or pressure on any of this of course, I managed to get something working for my application, it's more that I stumbled upon your project and thought it looked great compared to the alternatives, which caused me to roll my own. I just wanted to share that it could be more widely applicable (well, by +1) if the transport/client were flexible/customizable, since I can't use it in its current state for what I'm doing.

Sure. By the way, I think most of the types and traits for building your own XML-RPC client with custom transport protocol should be part of the public API already, but I could make anything that's missing for your use-case pub as well, if that would help.

blaenk commented 1 year ago

I didn't know about SCGI - is looks similar to HTTP?

Yeah the wiki page describes it pretty succinctly,

Example request:

"70:"
           "CONTENT_LENGTH" <00> "27" <00>
           "SCGI" <00> "1" <00>
           "REQUEST_METHOD" <00> "POST" <00>
           "REQUEST_URI" <00> "/deepthought" <00>
","
"What is the answer to life?"

So length of headers, colon, null-delimited header keys/values, comma, message. Just noticed they also include a method and uri header but that is not strictly required, that would depend on the application. In my case I only need CONTENT_LENGTH and SCGI as those I believe are mandated at a minimum.

Example response:

       "Status: 200 OK" <0d 0a>
       "Content-Type: text/plain" <0d 0a>
       "" <0d 0a>
       "42"

True, the current implementation only supports String as payload (i.e. valid UTF-8), because the XML de/serialization only works with UTF-8. Supporting arbitrary encodings or bytes is not going to be easy ... unless you'd implement custom XML de/serialization for every client. The client based on reqwest only supports UTF-8 right now, as that's what's supported by the serde feature of quick-xml.

Yeah no need to worry about encoding handling and all that, I meant more that if the api works with bytes then it becomes potentially future-proof and more general, and still works since a String can trivially be passed as bytes, especially if using one of the converter traits then both and more can be passed. Just an idea.

Sure. By the way, I think most of the types and traits for building your own XML-RPC client with custom transport protocol should be part of the public API already, but I could make anything that's missing for your use-case pub as well, if that would help.

That's a very good point! I actually was about to do that since it's more or less what I did with a combination of libraries, but I made a very shallow skim to see if the types/features I needed were public and I either got the impression that some pieces were not (may be way wrong here, couldn't list them for you now, so no worries on it) and/or I wasn't sure how inclined you would be to making necessary pieces public, so I just decided to go my own way for the time being.

However, at a later time I think I will revisit my implementation to see if I can define it in terms of this crate, and at that point I will be happy to provide feedback. Thanks again for working on this crate!

decathorpe commented 1 year ago

Sounds like a good plan! I'll close this ticket for now, feel free to re-open or open a new one once you're ready.