feat: multi-stage-output/new csv flags #1110

cristiand391 · 2024-10-31T19:29:24Z

What does this PR do?

Adds --line-ending flag to bulk commands that take a CSV file as input
update sf data delete/upsert bulk commands to use MSO

What issues does this PR fix or reference?

cristiand391 · 2024-11-11T15:27:09Z

src/bulkDataRequestCache.ts

+        options: {
+          connection: (await Org.create({ aliasOrUsername: entry.username })).getConnection(),
+        },
+      };


simplified resolveResumeOptionsFromCache so that it matches data import/export's cache resolver.
The previous implementation had a few unused properties and one (undocumented?) functionality:
passing a job ID that didn't exist in the cache wouldn't cause an error, the resolver would return it as if it was found in the local cache.
Added code ƒor specific bulk commands using this to avoid breaking changes.

cristiand391 · 2024-11-11T15:29:52Z

src/bulkIngest.ts

@@ -37,34 +38,45 @@ type ResumeCommandIDs = 'data import resume' | 'data update resume';
 *
 * It will create the specified bulk ingest job, set up the oclif/MSO stages and return the job info.
 * */
+// eslint-disable-next-line complexity


had to add this after adding the warning/deprecation/validation checks.

src/bulkIngest.ts

WillieRuemmele · 2024-11-12T20:23:42Z

src/bulkIngest.ts

+
+  if (opts.verbose && !['delete', 'hardDelete', 'upsert'].includes(opts.operation)) {
+    throw new SfError(
+      'Verbose mode is limited for `sf data delete/upsert bulk` for backwards-compat only and will be removed after March 2025.'


"backwards-compat" - @jshackell-sfdc

WillieRuemmele · 2024-11-12T20:26:27Z

src/bulkIngest.ts

  jobIdOrMostRecent: string | boolean;
  jsonEnabled: boolean;
  wait: Duration;
+  warnFn: (message: SfCommand.Warning) => void;


were you matching a pattern here? passing in a logFn/warnFn?

yep:

plugin-data/src/bulkIngest.ts

Line 57 in 3f72588

logFn: (message: string) => void;

to allow using command's this.log/warning methods here (need to wrap in arrow function for this scoping).

src/commands/data/export/resume.ts

WillieRuemmele · 2024-11-12T20:39:42Z

src/resumeBulkBaseCommand.ts

@@ -43,7 +43,7 @@ export abstract class ResumeBulkCommand extends SfCommand<BulkResultV2> {
      summary: messages.getMessage('flags.wait.summary'),
      unit: 'minutes',
      min: 0,
-      defaultValue: 0,
+      defaultValue: 5,


this will affect commands that don't currently pass --wait and that are expecting async behavior

True, but given the current behavior I think it's a better (and non-breaking) experience.

This base class is only used by sf data upsert/delete resume, without a default wait time these resume commands would return exit code 0 and suggest to re-run them again even if the job didn't finish:

data update/import resume also have --wait=5 as default.

cristiand391 · 2024-11-11T15:32:01Z

src/bulkIngest.ts

+  // validation
+  if (opts.externalId && opts.operation !== 'upsert') {
+    throw new SfError('External ID is only required for `sf data upsert bulk.');
+  }


inlined error msg b/c it's for devs only to avoid passing externalId when it's not needed.

cristiand391 · 2024-11-11T15:34:51Z

src/bulkIngest.ts

+    throw new SfError(
+      'Verbose mode is limited for `sf data delete/upsert bulk` for backwards-compat only and will be removed after March 2025.'
+    );
+  }


ditto, dev-only error to block --verbose on new bulk commands.

cristiand391 · 2024-11-11T15:36:57Z

src/bulkIngest.ts

+      columnDelimiter,
+    }).catch((err) => {
+      stages.stop('failed');
+      throw err;


chained catch to set failed stage properly (and avoid passing stages to createIngestJob)

cristiand391 · 2024-11-11T15:37:53Z

src/bulkIngest.ts

+      'Resuming a synchronous operation via `sf data upsert/delete resume` will not be supported after March 2025.'
+    );
+    await opts.cache.createCacheEntryForRequest(job.id, ensureString(conn.getUsername()), conn.getApiVersion());
+  }


related:
https://github.com/salesforcecli/plugin-data/pull/1110/files#r1836847805

cristiand391 · 2024-11-14T12:19:25Z

src/resumeBulkBaseCommand.ts

@@ -43,7 +43,7 @@ export abstract class ResumeBulkCommand extends SfCommand<BulkResultV2> {
      summary: messages.getMessage('flags.wait.summary'),
      unit: 'minutes',
      min: 0,
-      defaultValue: 0,
+      defaultValue: 5,


True, but given the current behavior I think it's a better (and non-breaking) experience.

This base class is only used by sf data upsert/delete resume, without a default wait time these resume commands would return exit code 0 and suggest to re-run them again even if the job didn't finish:

data update/import resume also have --wait=5 as default.

cristiand391 · 2024-11-14T12:21:40Z

src/bulkIngest.ts

  jobIdOrMostRecent: string | boolean;
  jsonEnabled: boolean;
  wait: Duration;
+  warnFn: (message: SfCommand.Warning) => void;


yep:

plugin-data/src/bulkIngest.ts

Line 57 in 3f72588

logFn: (message: string) => void;

to allow using command's this.log/warning methods here (need to wrap in arrow function for this scoping).

WillieRuemmele · 2024-11-26T17:19:28Z

QA Notes

on a call with Cristian to verify everything
✅ : flag deprecation warnings
✅ : resume an invalidated cache entry
✅ : json error information
✅ : auto-detect Column delims
✅ : using column delimiters
✅ : line endings

cristiand391 added 13 commits October 31, 2024 15:43

fix: cache refactor, remove unused code/props

5e2329f

chore: big refactor

2b99678

test: nuke UTs

2049e22

fix: keep cache entry

063d399

chore: more refactoring

6eeb736

fix: bring back verbose table

7c9ce9b

test: update HRO assertion

ac63a04

fix: bulk delete can return JSON failures

28f0da1

fix: match regex for table

41e3b32

fix: bulk upsert return json failures + NUT

6af6b5b

chore: remove unused code/refactor

4138648

fix: handle hardDelete missing perm error

07c944a

chore: address final TODOs

3f72588

cristiand391 marked this pull request as ready for review November 8, 2024 15:14

cristiand391 requested a review from a team as a code owner November 8, 2024 15:14

cristiand391 commented Nov 11, 2024

View reviewed changes

WillieRuemmele reviewed Nov 12, 2024

View reviewed changes

cristiand391 added 2 commits November 14, 2024 08:58

chore: code review

3244aba

chore: update msg

b323cc3

WillieRuemmele approved these changes Nov 14, 2024

View reviewed changes

cristiand391 commented Nov 14, 2024

View reviewed changes

cristiand391 added 2 commits November 14, 2024 14:08

chore: remove column-delimiter for bulk delete

63d887a

test: nuts use new flags

3c621b1

cristiand391 mentioned this pull request Nov 14, 2024

fix(http): handle more CSV delimiters on parse jsforce/jsforce#1622

Merged

cristiand391 added 3 commits November 15, 2024 11:41

fix: handle quoted fields when detecting column delimiter

79c7aa0

chore: removed dead code

803b7ac

chore: refactor

567bdf4

WillieRuemmele mentioned this pull request Nov 15, 2024

--line-ending does not seem to do anything forcedotcom/cli#3113

Closed

fix: remove ref to deleted md

cc9a20b

test: add import -> export -> import nut

f6e10b6

WillieRuemmele approved these changes Nov 25, 2024

View reviewed changes

cristiand391 added 4 commits November 25, 2024 19:02

fix: throw if job didn't process any record

7ef8d0e

fix: pass err action tokens

e52d2a4

fix: restore support for resuming non-local jobs

4db440e

fix: dont get records if job is in progress

6213764

cristiand391 added 2 commits November 26, 2024 14:40

fix: bump jsforce-node

95022b1

Merge remote-tracking branch 'origin/main' into cd/mso-new-flags

48cc145

cristiand391 merged commit 1d2cf30 into main Nov 26, 2024
27 checks passed

cristiand391 deleted the cd/mso-new-flags branch November 26, 2024 18:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: multi-stage-output/new csv flags #1110

feat: multi-stage-output/new csv flags #1110

cristiand391 commented Oct 31, 2024 •

edited

Loading

cristiand391 Nov 11, 2024

cristiand391 Nov 11, 2024

WillieRuemmele Nov 12, 2024

WillieRuemmele Nov 12, 2024

cristiand391 Nov 14, 2024

WillieRuemmele Nov 12, 2024

cristiand391 Nov 14, 2024 •

edited

Loading

cristiand391 Nov 11, 2024

cristiand391 Nov 11, 2024

cristiand391 Nov 11, 2024

cristiand391 Nov 11, 2024

cristiand391 Nov 14, 2024 •

edited

Loading

cristiand391 Nov 14, 2024

WillieRuemmele commented Nov 26, 2024

feat: multi-stage-output/new csv flags #1110

feat: multi-stage-output/new csv flags #1110

Conversation

cristiand391 commented Oct 31, 2024 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

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

cristiand391 Nov 14, 2024 • 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

cristiand391 Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillieRuemmele commented Nov 26, 2024

QA Notes

cristiand391 commented Oct 31, 2024 •

edited

Loading

cristiand391 Nov 14, 2024 •

edited

Loading

cristiand391 Nov 14, 2024 •

edited

Loading