Skip to content

fix(flags): deprecate async/wait flags without bulk#1002

Merged
mshanemc merged 6 commits intomainfrom
cd/cd/no-depends-on-bool-flag
Jul 26, 2024
Merged

fix(flags): deprecate async/wait flags without bulk#1002
mshanemc merged 6 commits intomainfrom
cd/cd/no-depends-on-bool-flag

Conversation

@cristiand391
Copy link
Copy Markdown
Member

@cristiand391 cristiand391 commented Jul 25, 2024

What does this PR do?

Deprecates using --wait and --async on data query without --bulk.
Can't remove because there a few thousands executions still running sf data query --wait, this will go through our deprecation policy.

➜  plugin-data git:(cd/cd/no-depends-on-bool-flag) ✗ ./bin/dev.js  data query -q 'select name,id from account limit 1' --wait 10
Warning: Using `--wait` without `--bulk` is deprecated and will be removed in a future release.
You can safely remove `--wait`, it never had any effect on the command without `--bulk`.

 NAME                       ID
 ────────────────────────── ──────────────────
 Bulk Account 1721852439393 0017e00001sQDkaAAG
Total number of records retrieved: 1.
Querying Data... done
➜  plugin-data git:(cd/cd/no-depends-on-bool-flag) ✗ ./bin/dev.js  data query -q 'select name,id from account limit 1' --async
Warning: Using `--async` without `--bulk` is deprecated and will be removed in a future release.
You can safely remove `--async`, it never had any effect on the command without `--bulk`.

 NAME                       ID
 ────────────────────────── ──────────────────
 Bulk Account 1721852439393 0017e00001sQDkaAAG
Total number of records retrieved: 1.
Querying Data... done
➜  plugin-data git:(cd/cd/no-depends-on-bool-flag) ✗

ignore this, we can't remove it now without breaking users.

Updates --wait and --async flags in data query to use oclif's relationship feature instead of dependsOn a boolean flag.
Found by the new eslint rule:
https://github.com/salesforcecli/eslint-plugin-sf-plugin/actions/runs/10097996137/job/27924166840?pr=447

This introduces a behavior change because both flags were depending on bulk which has default value (false), so the following scenarios were allowed accidentally:

sf data query -q 'select name from account limit 1' --async
sf data query -q 'select name from account limit 1' --wait 10

due to how data query conditionally queried the records, these commands also didn't fail/affect output:

const queryResult = flags.bulk
? await this.runBulkSoqlQuery(
conn,
queryString,
flags.async ? Duration.minutes(0) : flags.wait ?? Duration.minutes(0),
flags['all-rows'] === true
)
: await this.runSoqlQuery(
flags['use-tooling-api'] ? conn.tooling : conn,
queryString,
this.logger,
this.configAggregator.getInfo('org-max-query-limit').value as number,
flags['all-rows']

with this PR both examples will fail

TODO:
check telemetry, don't want to break a ton of users.

What issues does this PR fix or reference?

@W-16320656@

@cristiand391 cristiand391 changed the title fix(flags): no dependsOn boolean flags fix(flags): deprecate async/wait flags without bulk Jul 25, 2024
@cristiand391 cristiand391 marked this pull request as ready for review July 25, 2024 20:23
@mshanemc
Copy link
Copy Markdown
Contributor

QA:
✅ ran then command with the flag combos to see warnings

@mshanemc mshanemc merged commit d3dd4e9 into main Jul 26, 2024
@mshanemc mshanemc deleted the cd/cd/no-depends-on-bool-flag branch July 26, 2024 13:55
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.

2 participants