Open paul121 opened 9 months ago
@symbioquine I'm particularly curious your thoughts here. In some ways this is a departure from the structure you outlined in #31 (re: sync code generation, lower level transport), but it wasn't clear to me how some of the lower level transport methods would actually be implemented in that proposal. I like that the methods on the farmOS client would just be changed to async
/await
variants. I'm still a little confused if that would be compatible with some Twisted implementations, but I've read some things that describe how Twisted can leverage the (now) standard async/await syntax, perhaps with very little extra work.
Do you think we could encapsulate all of this complexity to a single client
parameter and still allow for a good amount of flexibility?
Let's decouple the transport backend from farmOS.py.
Resources
Original motivation from #31 which had some good resources:
I also enjoyed this article with some more recent info on state of async in Python: Asyncio, twisted, tornado, gevent walk into a bar...
Background
Currently we are using the requests
Session
object, via requests-oautlib, for all transport. This has worked well and should continue to work fine for many use-cases.But it would be great if farmOS.py could have async support, too. Async requests could be more efficient for use-cases that request lots of data from farmOS instances. It's also very import when making HTTP requests within an async web framework (like FastAPI).
For users of farmOS.py this would be opt-in and offer an API very similar to the standard synchronous API. As @symbioquine described in #31:
Implementation
The tricky part is implementation, and there are two parts.
First, we need farmOS clients that provide both synchronous and asynchronous methods. The main issue is that this could be twice as much code and more to maintain. #31 has an example implementation of how all logic could be written in async and have sync versions auto-generated. However since #31 and farmOS v2 w/ JSON:API, we have introduced a new generic
ResourceBase
class that contains nearly all of our client logic, and reduced quite a bit of the code in the library. We simply provide wrapper classes forlog
,asset
andterm
for convenience when using the client. I would be in favor of removing these wrapper classes entirely - they only provide little convenience, and end up abstracting important concepts (farmOS data model, JSON:API resources) that users should understand. This would reduce the amount of code to maintain to a minimum, enough where I would be OK having duplicate async/sync implementations to maintain, with possibility of automating in the future.Second, we need farmOS clients to actually implement async transport. This is is something that farmOS.py should not reinvent or prescribe to users. Ideally it could be provided via another library or custom implementation and brought in as needed. The only contract between the user and farmOS.py is that they provide a transport implementation that is compatible with the sync or async farmOS client they create.
I'm hopeful that the HTTPX client could basically be our recommended option for async. At minimum I want to make sure we design something that is compatible with it. The client is very modern, in active development (nearing a stable release) and most importantly has support for multiple async environments, including asyncio, Python's built-in library. This is one of the most fragmented parts of async in Python and the fact it supports multiple is a big win. I also like that it is largely compatible with Requests.
So... I'm starting to wonder if the abstraction for pluggable transport could be implemented similar to that for authentication #63. Instantiate and prepare a session/client object that is used when creating a farmOS client. What this means is that we're basically defining these abstractions around a session/client object very similar to Requests and HTTPX. It will be easy to use these clients, but others might require a wrapper class so they can behave "like requests". I think this would provide a good mix of easy to use, out of the box support, that still allows for flexibility and further customization.
One challenge would be documenting what this Requests interface looks like.. we might need to define a subset of this ourselves without depending on other libraries, including a few other things like the interface should return a
response
object that has a.json()
method so farmOS.py can handle them internally for some things like pagination. I think the good news is that farmOS.py shouldn't need require too many things, mostly just initiating requests with common parameters for URL, method, headers and data to send. Here are the existing API references: HTTPX Client and Requests SessionThis may look like: