Open paul121 opened 2 years ago
Preliminary testing of this is looking quite good! I'm actually able to use farmOS.py to make requests to the aggregator using api-key
authentication without any modifications to farmOS.py itself :tada:
This is the script I'm using for testing the new aggregator relay
endpoint: https://gist.github.com/paul121/17a3dda773c41074fc71128769cb2193
This works by creating a custom AggregatorSession
that subclasses the Requests session (similar to how requests_oauthlib
subclasses the requests
session). I then implement the methods our client
classes use: authorize
, http_request
and _http_request
:
authorize
: instead of dispatching an OAuth authorization, this just sets the api-key
header that will persist for all requests in the session. This could just be done in the __init__
method and avoid the client.authorize()
step altogether: https://gist.github.com/paul121/17a3dda773c41074fc71128769cb2193#file-aggregator_test-py-L19http_request
: modify to not include the hostname
in the path here, but keep api
base path_http_request
: copy logic from core farmOS.py session method, but add the api/v2/farms/resources/relay
base path: https://gist.github.com/paul121/17a3dda773c41074fc71128769cb2193#file-aggregator_test-py-L44Next, with this custom session created you can manually instantiate any of the client classes (resources, log, asset, etc):
hostname = 'http://localhost'
session = AggregatorSession(hostname)
session.authorize()
resources = ResourceBase(session)
And request logs from the farm_id=1
in the aggregator:
logs = resources.get('log', 'activity', {'farm_id': 1})
The only thing that does not work with this approach is the resource.iterate()
method. This is because it directly makes a request to the links.next
URL from the JSONAPI response: https://github.com/farmOS/farmOS.py/blob/086e30a5fd2e6e0dfe8090b0d08bfee60d943230/farmOS/client_2.py#L47
This is also using what is supposed to be the "private" _http_request
method. Works but just isn't flexible enough for this use case. Instead of directly using this next
url we should just use the path
and delegate to http_request
.
That way subsequent resource.iterate()
requests will go through the aggregator, instead of directly to the farmOS server.
** Fixed: https://github.com/farmOS/farmOS.py/commit/f13afa0450fe49144d28c60ff7dc133d47e29f2f
More generally, this gives me some ideas on ways to improve this client library, especially thinking back to #31 and ideas about supporting different session and/or sync/async libraries. The great thing is that, as demonstrated above, this is all largely possible right now by manually instantiating the client resource
class with a requests
compatible session object!
Thinking forward to a 1.0, 1.1 or maybe 2.0 release (I'm not sure how "breaking" we can be....) I'd like to explore further decoupling of the session and client classes. The client classes could really be tossed out to require uses instantiate separate "resource API" classes themselves - having separate log
, asset
, etc namespaces is a bit unnecessary anyways, at least until they provide unique additional functionality.
I'm particularly interested in the httpx library which is compatible with the existing requests
API but also supports (multiple) async environments
But anyways, as it pertains to this issue and aggregator support, none of this is required now. I'm glad our current abstractions will enable us to explore these ideas further without making changes now.
I then implement the methods our client classes use: authorize, http_request and _http_request:
Just want to flag this while it is fresh - these http_request
and _http_request
methods seem redundant. I think it might be an artifact of the 1.x implementation. But one main reason we need this helper method instead of calling session.request
or session.get
directly is because there is not a way to set a "base url" for the session to always use.
It might be possible to extend the parent session.request
method, or perhaps even the prepare_request
method. These might be more appropriate (and extendable) than using our own http_request
method.
It would be great if this library could also be used to communicate with the aggregator.
I am planning to add an additional "relay" endpoint on the aggregator that can be used to send a request at any given path (not limited to JSONAPI) to an individual farmOS server. In theory, this means farmOS.py just needs to send requests to a different api base path (
api/v2/relay
instead ofapi
) and be able to include additional query params to identify which farmOS server it intends to communicate with. The aggregator will authenticate these requests accordingly.farmOS.py will also need to support the
api-key
authentication that the aggregator uses. The aggregator is also its own OAuth provider (only supporting the password credentials grant), so perhaps farmOS.py could support that (if it doesn't already), but this is less of a priority.