feat: add support for merging strategy into the runner sdk #3420

TBonnin · 2025-01-31T01:24:50Z

adding setMergingStrategy runnder-sdk function
- if strategy is ignore-if-modified-since the function calls persist /cursor and store it in a private variable. Its value is then forwarded when calling batchSave/Update/Delete which are now sending the next cursor. Again the updated cursor is forwarded to batchSave...
- One limitation of this approach is that it makes concurrent batchSave undefined behavior. We already recommend not to do that anyway because it doesn't play well with rate-limiting
refactor persist API related code in runner-sdk into its own file
send merging strategy to persist and handle response with next merging strategy with new cursor
fix a couple of bugs from previous PR
modify CLI compiler to disallow setting merging strategy multiple times or setting it after bachSave

linear · 2025-01-31T01:24:53Z

TBonnin · 2025-01-31T02:15:39Z

packages/cli/lib/services/parser.service.ts

@@ -70,15 +70,18 @@ class ParserService {
            'triggerAction'
        ];

-        const disallowedActionCalls = ['batchSend', 'batchSave', 'batchDelete'];
+        const disallowedActionCalls = ['batchSend', 'batchSave', 'batchDelete', 'batchUpdate'];


unrelated but batchUpdate was missing

TBonnin · 2025-01-31T02:16:47Z

packages/cli/lib/services/parser.service.ts

+                                    );
+                                    usedCorrectly = false;
+                                }
+                            }


adding support for functions like setMergingStrategy that take multiple models as argument

TBonnin · 2025-01-31T02:17:57Z

packages/persist/lib/routes/environment/environmentId/connection/connectionId/getCursor.ts

-    Success: {
-        cursor?: string;
-    };
+    Success: GetCursorSuccess;


moving the type to types package so it can be used by the runner-sdk

TBonnin · 2025-01-31T02:19:16Z

...b/routes/environment/environmentId/connection/connectionId/sync/syncId/job/jobId/validate.ts

@@ -8,7 +8,7 @@ const mergingStrategySchema = z.discriminatedUnion('strategy', [
    }),
    z.object({
        strategy: z.literal('ignore_if_modified_after_cursor'),
-        cursor: z.string()
+        cursor: z.string().optional()


forgot the scenario in my previous PR where there is not records yet (first sync or after emptying the cache). In this case there is no cursor yet

TBonnin · 2025-01-31T02:19:58Z

...b/routes/environment/environmentId/connection/connectionId/sync/syncId/job/jobId/validate.ts

@@ -31,8 +32,7 @@ export const validateRecords = <E extends Endpoint<any>>() =>
                    environmentId: z.coerce.number().int().positive(),
                    nangoConnectionId: z.coerce.number().int().positive(),
                    syncId: z.string(),
-                    syncJobId: z.coerce.number().int().positive(),
-                    merging: mergingStrategySchema.default({ strategy: 'override' })
+                    syncJobId: z.coerce.number().int().positive()


TBonnin · 2025-01-31T02:22:34Z

packages/records/lib/models/records.ts

+                    .orderBy([
+                        { column: 'updated_at', order: 'asc' },
+                        { column: 'id', order: 'asc' }
+                    ]);


insertion order is different from cursor order since cursor is based on id which is a generated uuid. so we need to order based on cursor to get the latest cursor below

TBonnin · 2025-01-31T02:22:55Z

packages/records/lib/types.ts

-
-export type MergingStrategy = { strategy: 'override' } | { strategy: 'ignore_if_modified_after_cursor'; cursor: string };
-
-export type CursorOffset = 'first' | 'last';


moved to types package

TBonnin · 2025-01-31T02:24:41Z

packages/runner-sdk/lib/action.ts

-            return value;
-        });
-    }
-


not necessary anymore. Was used by the persistApi related code in NangoSync runner-sdk class.
PersistApi related code now lives in its own file and the secret is not logged anymore

TBonnin · 2025-01-31T02:26:18Z

packages/runner/lib/sdk/persist.ts

+import type { Result } from '@nangohq/utils';
+import type { CursorOffset, DeleteRecordsSuccess, GetCursorSuccess, MergingStrategy, PostRecordsSuccess, PutRecordsSuccess } from '@nangohq/types';
+
+export interface PersistApi {


persist related code in runner-sdk NangoSync was starting to get out of control. Moving all the persist related code to its own file

TBonnin · 2025-01-31T02:27:37Z

packages/types/lib/persist/api.ts

+
+export interface DeleteRecordsSuccess {
+    nextMerging: MergingStrategy;
+}


I could have a single one. wdyt?

TBonnin · 2025-01-31T03:05:12Z

packages/cli/lib/services/parser.service.ts

+        const callsReferencingModelsToCheck = callsBatchingRecords.concat('setMergingStrategy');
+        const proxyLines: number[] = [];
+        const batchingRecordsLines: number[] = [];
+        const setMergingStrategyLines: number[] = [];


see below. addressing footguns by raising compilation errors and enforce a couple of rules:

setMergingStrategy must be called only once

setMergingStrategy must be called before any batch operation

setMergingStrategy must be called before any proxy/get/post/...

TBonnin force-pushed the tbonnin/nan-2415/merging-sdk branch from 905b0c8 to b1c0f9a Compare January 31, 2025 02:02

TBonnin commented Jan 31, 2025

View reviewed changes

TBonnin marked this pull request as ready for review January 31, 2025 02:31

TBonnin requested a review from a team January 31, 2025 02:32

feat: add support for merging strategy into the runner sdk

b5c60d6

TBonnin force-pushed the tbonnin/nan-2415/merging-sdk branch from b1c0f9a to b5c60d6 Compare January 31, 2025 03:02

TBonnin commented Jan 31, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for merging strategy into the runner sdk #3420

feat: add support for merging strategy into the runner sdk #3420

TBonnin commented Jan 31, 2025 •

edited

Loading

linear bot commented Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025

TBonnin Jan 31, 2025


		export type MergingStrategy = { strategy: 'override' } \| { strategy: 'ignore_if_modified_after_cursor'; cursor: string };

		export type CursorOffset = 'first' \| 'last';

feat: add support for merging strategy into the runner sdk #3420

Are you sure you want to change the base?

feat: add support for merging strategy into the runner sdk #3420

Conversation

TBonnin commented Jan 31, 2025 • edited Loading

linear bot commented Jan 31, 2025

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

TBonnin commented Jan 31, 2025 •

edited

Loading