Skip to content
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

Add UCR support #222

Merged
merged 3 commits into from
Sep 5, 2023
Merged

Add UCR support #222

merged 3 commits into from
Sep 5, 2023

Conversation

Charl1996
Copy link
Contributor

Ticket

Background:
This work adds support to the DET for exporting data sources from CommCareHQ using the api_get_ucr_data API endpoint.

Due to the generic nature of the DET, not much work was needed to make the UCR exports compatible.

One thing to note:
The datasources on HQ is stored in a custom table. This table does not have an id column and uses the doc_id column as the primary key. When HQ gives the datasource data it adds an id column (in addition to the doc_id column) which contains the same value as the doc_id column, but this new id column is the primary key in the DET database. If this is a problem I can change that, but that will require some additional changes to the DET.

Copy link
Contributor

@snopoke snopoke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this implementation the DET will not record any checkpoints (checkpoints currently only support the DatePaginator - see here). This means that each run of the DET will attempt to re-export the full UCR table.

To support checkpoints I think we could do one of 2 things:

  1. (preferred) Update the checkpoint to allow storing the 'cursor' value
  2. Use our knowledge of the internals of the API create a new 'DateCursorPaginator' which extracts the date and doc ID from the cursor to store in the checkpoint.

Note that we don't currently use 'checkpoint.last_doc_id' at all. If we want to use it in pagination we'll need to make some other code changes to pass it (or the whole checkpoint) through.

@Charl1996
Copy link
Contributor Author

Charl1996 commented Aug 29, 2023

@snopoke Thanks for highlighting that (and sorry for only coming back to you now).

I'm almost done adding the checkpoint support (draft PR). For now, though, I'm officially opening this PR for review.

@Charl1996 Charl1996 marked this pull request as ready for review August 29, 2023 09:43
Copy link
Contributor

@snopoke snopoke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, #224 should fix the build

@Charl1996 Charl1996 merged commit 4684b51 into master Sep 5, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants