[BACK-43] Dedup hash val #195

jh-bate · 2023-10-04T23:19:09Z

generate platform style dedup hash for migration

NOTE:

supersedes [BACK-43] include deduplicator for datum #191 that was truncating values

use toISOString() from time field

* updates to match platform unit conversion precision * cleanup test * updates for bg conversion precision

darinkrauss · 2023-10-05T00:03:09Z

lib/schema/urineKetone.js

@@ -21,6 +21,7 @@ var schema = require('./schema.js');

 var idFields = ['type', 'deviceId', 'time'];
 schema.registerIdFields('urineKetone', idFields);
+schema.registerFieldsForDuplicator('urineKetone');


FYI - I don't think platform handles this data type. If Uploader sends this data type, we'll need to update platform. Please note this in the narrative.

Uploader does not use urineKetone, only bloodKetone

darinkrauss · 2023-10-05T00:04:55Z

lib/schema/note.js

@@ -21,6 +21,7 @@ var schema = require('./schema.js');

 var idFields = ['type', 'time', 'creatorId', 'text', 'deviceId'];
 schema.registerIdFields('note', idFields);
+schema.registerFieldsForDuplicator('note');


FYI - I don't think platform handles this data type. If Uploader sends this data type, we'll need to update platform. Please note this in the narrative.

Uploader does not use note, but I think Tidepool Mobile and Blip does? Then again, they're probably both using Platform, right?

darinkrauss · 2023-10-05T00:14:18Z

test/api/upload/output.json

-        "_active": true
+        "_active": true,
+        "_deduplicator": {
+            "name": "org.tidepool.deduplicator.device.deactivate.hash",


We'll need to make sure that by using this deduplicator name it isn't going to cause an issue with future uploads via platform. I think just getting this in the narrative for now is good.

darinkrauss · 2023-10-05T00:18:26Z

package-lock.json

@@ -1,8 +1,7624 @@
 {


We should back out any changes to the lock files for this PR and use the other PR updating the actual dependencies to update the lock files.

yeah I will merge it in

darinkrauss · 2023-10-05T00:19:03Z

yarn.lock

@@ -3,4203 +3,4205 @@



We should back out any changes to the lock files for this PR and use the other PR updating the actual dependencies to update the lock files.

darinkrauss · 2023-10-09T19:58:40Z

lib/schema/duplicate.js

@@ -0,0 +1,77 @@
+'use strict';


[nit] Why is this file called duplicate.js? Perhaps, platform.js or similar to indicate it is the platform deduplicator? That way when it has used it would be platform.registerFieldsForDuplicator which easily shows it is for platform.

Or, perhaps include "platform" in the exported functions so it is clear that it is for platform deduplication.

darinkrauss · 2023-10-09T20:00:24Z

lib/schema/duplicate.js

+  }
+  var idHashFields = idHashFieldMap[datum.type];
+  if (!idHashFields) {
+    return '';


Does the caller check for empty string and not write the platform deduplicator hash? We don't want any with empty strings in the database I think.

darinkrauss · 2023-10-09T20:10:45Z

lib/schema/duplicate.js

+    if (idHashFields[i] === 'time') {
+      const dateTime = new Date(val);
+      // NOTE: platform `time` is being returned minus millis so we need to do the same here
+      val = dateTime.toISOString().split('.')[0] + 'Z';


Huh, I didn't know this. And it has the Z timezone at the end?

Do we need to convert timezones here?

darinkrauss · 2023-10-09T20:11:14Z

lib/schema/duplicate.js

+        datum.type === 'cbg'
+      ) {
+        // NOTE: platform `value` precision is being used so that the hash will be the same
+        if (val.toString().length > 7) {


I think we only want to do this for mg/dL per our platform discussion.

darinkrauss · 2023-10-09T20:18:03Z

lib/schema/schema.js

@@ -513,3 +521,7 @@ exports.makeId = function(datum) {

  return exports.generateId(datum, idFields);
 };
+
+exports.generateHash = duplicate.generateHash;


Perhaps include "platform" name in exports so it is clear it is for the platform deduplication.

jh-bate · 2023-11-30T21:17:29Z

/deploy qa3

jh-bate · 2023-11-30T21:55:45Z

/deploy qa3

tidebot · 2023-11-30T21:55:50Z

jh-bate updated values.yaml file in qa3

tidebot · 2023-11-30T21:55:51Z

jh-bate updated flux policies file in qa3

tidebot · 2023-11-30T21:55:52Z

jh-bate deployed jellyfish dedup-hash-val branch to qa3 namespace

jh-bate · 2023-12-01T09:55:55Z

/deploy qa3

tidebot · 2023-12-01T09:56:00Z

jh-bate updated values.yaml file in qa3

tidebot · 2023-12-01T09:56:02Z

jh-bate updated flux policies file in qa3

tidebot · 2023-12-01T09:56:02Z

jh-bate deployed jellyfish dedup-hash-val branch to qa3 namespace

darinkrauss

Just a couple more things that may need to be addressed.

darinkrauss · 2024-04-26T23:47:00Z

lib/schema/platform.js

+    if (idHashFields[i] === 'time') {
+      const dateTime = new Date(val);
+      // NOTE: platform `time` is being returned minus millis so we need to do the same here
+      val = dateTime.toISOString().split('.')[0] + 'Z';


If, for some reason there isn't a ., then it will end up with and extra Z. For example "2011-10-05T14:48:00Z" becomes "2011-10-05T14:48:00ZZ", I think.

darinkrauss · 2024-04-26T23:48:26Z

lib/schema/platform.js

+      ) {
+        // NOTE: platform `value` precision is being used so that the hash will be the same
+        if (val.toString().length > 7) {
+          val = convertMgToMmol(val);


So, we aren't actually truncating the value in the datum, but computing it separately just for the hash. I think the current migrator is actually persisting the truncated value in the datum, right? Is that an issue?

jh-bate · 2024-09-17T01:29:35Z

superseded by #203

jh-bate and others added 24 commits August 24, 2023 14:57

include _deduplicator for upload type

0b906be

add _deduplicator to output

da75a44

deduplicator for all types

69c7645

remove empty hash

f63537d

bson format

47b7e21

omit deduplicator

90c4a47

Merge branch 'master' into add-platform-deduplication-hash

298d146

test mongo changes

639286a

_deduplicator for all types

50759a8

generate hash as per platform

b81bf9e

formatting

d8217eb

update to use the registered idFields for each data type

d2103b0

register for each individual type

c000e04

formatting

f804770

sanity tests for datum hash

e57f538

use toISOString() from time field

test fixes

549aef6

formatting

8a8ff60

add tests that are direct comparison with what platform returns

60e3601

fix test

32473a2

[BACK-43] Add platform style conversion (#193)

d58fa99

* updates to match platform unit conversion precision * cleanup test * updates for bg conversion precision

fix conversion rounding

c11f9d2

only upload has name and version _deduplicator detail

6528cb4

do not truncate value

80cc1b9

add test for truncated val used in hash

0d89f0b

jh-bate requested a review from darinkrauss October 4, 2023 23:34

darinkrauss reviewed Oct 9, 2023

View reviewed changes

jh-bate added 3 commits October 11, 2023 12:18

naming updates from review

572050b

updates to error if type not registered for platform hash

24a6d7a

remove unreachable code

4d8b48a

jh-bate changed the title ~~Dedup hash val~~ [BACK-43] Dedup hash val Oct 11, 2023

jh-bate mentioned this pull request Oct 11, 2023

[BACK-43] include deduplicator for datum #191

Closed

toddkazakov mentioned this pull request Oct 30, 2023

[BACK-2702] Allow closing datasets created by jellyfish so we can recalculate summaries correctly tidepool-org/platform#677

Draft

jh-bate added 3 commits November 8, 2023 12:38

Merge branch 'master' into dedup-hash-val

986c7fa

tweak to provide upload destination

21625df

Merge branch 'master' into dedup-hash-val

a42583d

sp

5da25ed

jh-bate added 2 commits April 4, 2024 13:48

Merge branch 'master' into dedup-hash-val

6ab36c1

remove commented out code for summary tests

618c957

jh-bate requested a review from darinkrauss April 9, 2024 05:19

darinkrauss reviewed Apr 26, 2024

View reviewed changes

jh-bate closed this Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BACK-43] Dedup hash val #195

[BACK-43] Dedup hash val #195

jh-bate commented Oct 4, 2023 •

edited

Loading

darinkrauss Oct 5, 2023

gniezen Oct 16, 2023

darinkrauss Oct 5, 2023

gniezen Oct 16, 2023

darinkrauss Oct 5, 2023

darinkrauss Oct 5, 2023

jh-bate Oct 10, 2023

darinkrauss Oct 5, 2023

jh-bate Oct 10, 2023

darinkrauss Oct 9, 2023

darinkrauss Oct 9, 2023

darinkrauss Oct 9, 2023

darinkrauss Oct 9, 2023

darinkrauss Oct 9, 2023

darinkrauss Oct 9, 2023

darinkrauss Oct 9, 2023

jh-bate commented Nov 30, 2023

jh-bate commented Nov 30, 2023

tidebot commented Nov 30, 2023

tidebot commented Nov 30, 2023

tidebot commented Nov 30, 2023

jh-bate commented Dec 1, 2023

tidebot commented Dec 1, 2023

tidebot commented Dec 1, 2023

tidebot commented Dec 1, 2023

darinkrauss left a comment

darinkrauss Apr 26, 2024

darinkrauss Apr 26, 2024

jh-bate commented Sep 17, 2024

[BACK-43] Dedup hash val #195

[BACK-43] Dedup hash val #195

Conversation

jh-bate commented Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jh-bate commented Nov 30, 2023

jh-bate commented Nov 30, 2023

tidebot commented Nov 30, 2023

tidebot commented Nov 30, 2023

tidebot commented Nov 30, 2023

jh-bate commented Dec 1, 2023

tidebot commented Dec 1, 2023

tidebot commented Dec 1, 2023

tidebot commented Dec 1, 2023

darinkrauss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jh-bate commented Sep 17, 2024

jh-bate commented Oct 4, 2023 •

edited

Loading