OpenFn / adaptors

The new home for OpenFn adaptors; re-usable connectors for the most common DPGs and DPI building blocks.
GNU General Public License v3.0
4 stars 8 forks source link

ODK: Update API #654

Closed josephjclark closed 3 days ago

josephjclark commented 1 week ago

Summary

In response to comments from Yaw at ODK, I've given the ODK adaptor (another) once over

Details

There are a number of changes here:

Unit tests have been rewritten.

Docs have been overhauled and now include state tables

image

Issues

652 #653

josephjclark commented 1 week ago

I want to build in a fix to #653 now - that's actually really easy because we can just feed the parameters right through to request. I'll do that in the morning and add a test.

josephjclark commented 1 week ago

@taylordowns2000 getForms is back ( I see the difference now!)

Following from our chat about callbacks today, I want to remove all the callbacks from this API. That means I can add query support to getSubmissions, which could be neat.

Any objections?

The runtime doesn't support everything as a promise yet, but if I prioritise it I think we can have that released next week. So it's not far away!

josephjclark commented 1 week ago

@taylordowns2000 Alright this is all ready to go!

yanokwa commented 3 days ago

Thanks for the great work so far, @josephjclark!

One potentially confusing thing about this API is that getSubmissions returns submission data and getForms returns form metadata. How about matching the naming we use in pyODK, our Python API client?

Here's what I'd propose...

listForms(projectId) List all Form metadata in a project

listSubmissions(projectId, xmlFormId) List all Submission metadata on a Form

getSubmissionsTable(projectId, xmlFormId, tableName, skip, top, count, wkt, filter, expand, select) Get submission data as an OData JSON document.

For getSubmissionsTable, it would be helpful to have the tableName be something that can be passed in. It would be fine to keep the other query parameters raw as you have them now if that feels more in line with typical OpenFn adapters.

Is there something that can be done at the adaptor level to help with automatically filtering to only receive newer submissions? Ideally it would be really easy for users to use the latest __system/submissionDate from a job run to filter the requests in the next run. https://docs.openfn.org/documentation/build/triggers#cron-triggers-formerly-timers mentions “keeping a cursor” but it looks like that section is missing. Is setting what final_state should be something that adaptors can do?

josephjclark commented 3 days ago

Appreciate the input Yaw :pray:

I've raised an issue to easily filter by date. That's a great use-case and I can see it's hard to do in the API right now. We'll address that - it's very fixable. Thanks!

I'm not so sure about matching the pydoc though. In OpenFn we regularly use the word get and I'd be keen to stick with that verb across the APIs, unless there's a really compelling reason to change.

I've had a chat with Taylor and based on past ODK/OpenFn projects, we know our users will need to getForms to find the formIDs for a project and then getSubmissions to extract the actual submission data (not the submitter metadata.)

That's the pattern we want to lean into.

We know a lot less about your user’s integration needs than you do! But we're not sure when it's valuable to get the metadata alone for forms or submissions? And isn't the metadata returned through the odata endpoint?

So we'd like to stick with getForms and getSubmissions and drop the metadata API.

If you think there's a compelling use-case for metadat only calls, I'd want do add an option to all functions in the adaptor - something like this would feel reasonable and consistent:

getForms(projectId), getSubmissions(projectId, formId, { metaOnly: true })
yanokwa commented 3 days ago

I'm very happy to defer to your conventions and patterns!

Let's get this PR merged as-is and get started on the filter by date.