TrillionTrees: many:many tables not populating

ritazagoni commented 3 years ago

Describe the bug

The external many:many tables TT_SiteRegistrationAnimalActionShift, TT_SiteRegistrationTreeType and TT_SiteRegistrationAnimal are not being populated

To Reproduce

Here is a link to a failed run on OpenFn.org wihch is indicative of the bug: https://www.openfn.org/projects/trillion-trees/runs/061967f0-4344-7f27-9c2e-3298d27c57d3

Diane reported that the TT_SiteRegistrationAnimalActionShift, TT_SiteRegistrationTreeType and TT_SiteRegistrationAnimal were not populated even before the job started failing.

Please consider (1) what changes need to be made to this job so it works, as well as (2) if there are related changes that need to be made to the entire automation solution.

expression.js

Link to the job itself in Github: https://github.com/WCS-ConsTech/OpenFn-TrillionTrees/blob/main/TT_SiteRegistration.js

state.json

See LP WCS Mssql TT DB https://www.openfn.org/projects/trillion-trees/messages/0618bdbd-b24b-76f4-ac55-361d4e818814

To test/resolve

Feel free to re-run https://www.openfn.org/projects/trillion-trees/runs/061967f0-4344-7f27-9c2e-3298d27c57d3

aleksa-krolls commented 3 years ago

@ritazagoni looking good, just added the comment in bold.

@lakhassane please pick this up tomorrow... and I've added you to the WCS call invite. Free to join for 5-10 min?

ritazagoni commented 3 years ago

@lakhassane please see the new location for the job: https://github.com/WCS-ConsTech/OpenFn-TrillionTrees/blob/main/TT_SiteRegistration.js

lakhassane commented 3 years ago

Hey @aleksa-krolls the job is "almost" working. Can you please whenever possible look at line 243 here https://openfn.org/projects/trillion-trees/jobs/jyz8jn and advise what should be used for uuid? Or should we ask Diane?

aleksa-krolls commented 3 years ago

@lakhassane were you able to talk to Steve? What's the status? Please try to let us know where things sit before signing off so @ritazagoni and I can support. Thanks!

aleksa-krolls commented 3 years ago

@lakhassane I know you had a lot going on yesterday... but how did this finish up? What's the latest status to pick up on Monday? (I want to make sure I can update WCS... thanks!)

lakhassane commented 3 years ago

@aleksa-krolls sorry yes, I pushed the changes done yesterday. But there are still mappings that need review from WCS (the last 3 operations of the job). I comment out TT_SiteRegistrationID awaiting review

aleksa-krolls commented 3 years ago

Thanks @lakhassane... @ritazagoni can you review and follow up with WCS and Mamadou as needed?

aleksa-krolls commented 3 years ago

@lakhassane Two pieces of feedback to help you finish this...

Should this line reference a specific Kobo question ( where_animals_shifted) and not just state.data? https://github.com/WCS-ConsTech/OpenFn-TrillionTrees/blob/main/TT_SiteRegistration.js#L234

To link m:m tables to TT_SiteRegistrationID, the mapping should look like this...

TT_SiteRegistrationID: await findValue({
      uuid: 'TT_SiteRegistrationID',
      relation: 'TT_SiteRegistration',
      where: { AnswerId: state.data._id },
    })(state),

We always will use find value when upserting these m:m tables... Screen Shot 2021-11-29 at 10.45.37 AM.png

lakhassane commented 3 years ago

@aleksa-krolls ...

In this specific case the value of where_animals_shifted is inside state.data that's why.
I used the findValue so having a green check here: https://www.openfn.org/projects/trillion-trees/runs/061a4c5b-3a2d-76f4-8a7c-ada6770d56d3

But note that the last 3 operations in the job are skipped because there is no array for upsert. So, we might need another message for more deep testing.

aleksa-krolls commented 3 years ago

Thanks, @lakhassane. Not sure this is working... Seeing errors since making these changes: https://www.openfn.org/projects/trillion-trees/runs/061a4c94-3258-7557-8769-837d32ff5e2f https://www.openfn.org/projects/trillion-trees/runs/061a4c99-1d2f-77b9-8fd4-4f119876418b

aleksa-krolls commented 3 years ago

@lakhassane We're encountering this error when trying to use the m:m table PK e.g., upsert onTT_SiteRegistrationAnimalActionShiftID

RequestError: Cannot update identity column 'TT_SiteRegistrationAnimalActionShiftID'.

Let's add a varchar GeneratedUuid column with a unique constraint to every one of these m:m tables so that we can upsert these rows.

aleksa-krolls commented 3 years ago

@lakhassane as discussed with Steve... we will upsert the m:m tables via a unique constraint that combines the IDs of the 2 parent tables.

Steve confirmed that these have now been added added to the m:m tables. Can you please review, change the upsert keys in the job, and test to see if this works now?

The 3 TT m:m tables now have a unique constraint on 
TT_SiteRegistrationID and their respective other IDs 
(TT_AnimalID, TT_AnimalActionShiftID,TT_TreeTypeID)

lakhassane commented 3 years ago

@aleksa-krolls yes that was super helpful.

mssql is different than postgresql as it does not allow to specify the name of the constraint for upsert. So I made a modification to the adaptor to allow multiple uuids at the same time. Check here

So from now on, we can use either

With only one uuid

upsertMany(
'TableName',
'uuid',
() => mapping,
)

With multiple uuids
```
upsertMany(
'TableName',
['uuid1', 'uuid2'],
() => mapping,
)
```
Note that this is valid only on mssql Note2: autodeploy is still not activated in the job in prod to sync with GH

lakhassane commented 3 years ago

@ritazagoni once @taylordowns2000 has deployed v2.6.10 for mssql can you update the job adaptor version and test please?

lakhassane commented 3 years ago

@ritazagoni I tested with v2.6.11, find the runs for the latest failed runs. run 1: https://www.openfn.org/projects/trillion-trees/runs/061a75b4-71b9-7d17-bb47-ff3cbfe68432 run 2: https://www.openfn.org/projects/trillion-trees/runs/061a75b5-51d2-7ded-8627-b479fca43909

aleksa-krolls commented 3 years ago

Thanks @lakhassane ... did a bulk reprocess and everything is passing nicely! Will let you know if any further feedback.

OpenFn / ConSoSci