-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Provide API for working with farmOS Aggregator #35
Comments
Bumping this up to the beta milestone, so we can look at this at the same time we address farmOS/field-kit#452, which will need to be resolved by then. |
I pushed this back to the general release milestone a few weeks ago, and still don't want this to block the beta release, which will hopefully happen later today; however, I believe we may move more quickly on this than I originally expected, depending on when this is needed for integration with SurveyStack. Also, @paul121, do I recall you saying yesterday that there were still some changes that needed to happen on the Aggregator itself, to enable relaying requests to the individual farmOS instances? |
I spoke a bit with @paul121 about how best to implement this, and to copy and past liberally from that convo... I think for the case of the Aggregator, since it's already so close to the farmOS API, we can probably just drop in (or pass in) a different authorization method for the client itself. The OAuth stuff is all pretty much isolated to Line 68 in fd2d839
So to extend the const auth = apiKeyAuth;
const farm = client(host, {
clientId, getToken, setToken, auth,
}); where const apiKeyAuth = (request, authOpts = {}) => {
request.interceptors.request.use(/** set up API key access */);
request.interceptors.response.use(/** additional handlers */);
return {
autorize() {
// etc...
},
};
}; The basic idea I'd like to develop for the Ideally the I think there are relatively few changes we need to make within farmOS.js to make this possible, so @paul121 can begin testing it out in his local Aggregator dev environment by next week. |
Okay, so I've parameterized OAuth as the default /**
* @typedef {Function} AuthMixin
* @param {import('axios').AxiosInstance} request
* @param {Object} authOptions
* @property {String} authOptions.host
* @returns {Object<string,function>}
*/ Hopefully that should be enough to get you started, @paul121! |
And I just tagged these changes as 2.0.0-beta.6, so if you install the |
Blah, I accidentally broke HEAD with 2.0.0-beta.6 and didn't notice til I deployed it to develop.farmos.app, so make sure to use 2.0.0-beta.7. |
A little overdue, but this is working! These changes make it possible to create a farmOS.js client that uses an I have an example of this here, eventually we should include this in the aggregator docs directly: https://gist.github.com/paul121/26bed0987b73c6886fa3a0743c0f47eb @jgaehring I think we can close this issue :-) |
Oh awesome! We could include something in the farmOS.js docs, too! Maybe just a link to the Aggregator docs? |
In discussion today, @mstenta mentioned that Our-Sci (and potentially other partners working on integrations with farmOS) will soon have a need to make calls to a farmOS Aggregator service in the very near future. I don't think it makes sense to provide such an API in farmOS.js for 1.x, which is effectively deprecated, but it would be nice to have this ready by the general release of 2.0.0 at the latest. With the changes I've made to the
connect
portion of the library, particularly in how the OAuth portions are separated into their own functions, I hope this will not require too much effort.A first step, at least for myself, would be to get better acquainted with the Aggregator's own API, and how this can be dovetailed with the work already done with the farmOS instance API. Perhaps @paul121 and I can sit down at some point to take a look at this together.
The text was updated successfully, but these errors were encountered: