feat(dev-env): add quiet mode for "import sql"

[sjinks]

Description

Related: #1561

This PR adds:

• --skip-reindex flag to disable reindexing (like other boolean options, it can be negated to request reindexing: --no-skip-reindex explicitly). The default is to reindex if the WP CLI vip-search command is available;
• --quiet flag to suppress extra output from the script (in addition, in this mode, --quiet is passed to WP CLI, and --silent is passed to mysql).

These options can be combined together.

Pull request checklist

• Update SETUP.md with any new environmental variables.
• Update the documentation.
• Manually test the relevant changes.
• Follow the pull request checklist
• Add/update automated tests as needed.

New release checklist

• Automated tests pass.
• The Preparing for release checklist is completed.

Steps to Test

See #1561

vip dev-env create --elasticsearch true < /dev/null
vip dev-env start
vip dev-env exec -q -- wp db export - > data.sql
# This command will show more information
vip dev-env import sql data.sql
# This one won't
vip dev-env import sql data.sql -q
# This will suppress reindexing
vip dev-env import sql data.sql --skip-reindex

[sjinks]

@yolih vip dev-env import sql will have an additional option:

-q, --quiet           Suppress prompts.

[chriszarate]

@sjinks Would a flag that controls ES reindexing (--reindex-elasticsearch=<yes|NO>) be more useful? A --quiet option removes the choice of whether or not to reindex and might be another blocker to full automation.

Alternatively, we could make reindexing the default and introduce a --skip-reindex-elasticsearch option.

[sjinks]

@chriszarate I have just added the --[no-]skip-reindex option.

So,

1. If --skip-reindex (or --skip-reindex false|0|n|no) is specified, the index prompt is not shown, and reindexing is not performed.
2. If --no-skip-reindex is specified, the index prompt is not shown, and reindexing is performed.
3. If --quiet is specified, no questions are asked, and reindexing is performed (the same behavior as when the stdin is not a TTY).

[chriszarate]

I'm not quite seeing the value of --quiet, especially when it hides a default behavior from the user. In fact, I'm sort of questioning the value of the prompt entirely. Why not have a sensible default behavior (reindex when ES is enabled) and a simple --skip-reindex to override it?

If --no-skip-reindex is specified, the index prompt is not shown, and reindexing is performed.

This appears to have been left out, which is a good choice IMO.

[sjinks]

I'm not quite seeing the value of --quiet, especially when it hides a default behavior from the user.

--quiet does a bit more than that. It also hides informational messages (such as "Check for Docker installation," "Check for docker-compose installation," etc) so that the script can be safely used with cron. --quiet assumes a non-interactive environment and assumes the default behavior. In brief, --quiet is the same as --no-skip-reindex with informational messages hidden.

[chriszarate]

In brief, --quiet is the same as --no-skip-reindex with informational messages hidden.

I get that, but this behavior is hidden and not apparent from the help text. If I were writing automations using vip, I would certainly want a way to suppress interactivity.

But more importantly I would want clear, documented flags that let me completely control the behavior of the command. In other words, any interactive prompt should have a corresponding flag that lets me select all possible choices non-interactively.

I like this resource and its principles:

Only use prompts or interactive elements if stdin is an interactive terminal (a TTY). This is a pretty reliable way to tell whether you’re piping data into a command or whether it’s being run in a script, in which case a prompt won’t work and you should throw an error telling the user what flag to pass.

https://clig.dev/#interactivity

At any rate, I don't want to stand in the way of overall improvement, so the PR is approved and I'll let you decide the path forward.

[aswasif007]

This is gonna be an issue:

vip-cli/src/commands/dev-env-sync-sql.js

Lines 177 to 182 in 054cd85

	const importOptions = {
	inPlace: true,
	skipValidate: true,
	};
	const importCommand = new DevEnvImportSQLCommand( this.sqlFile, importOptions, this.slug );
	await importCommand.run( true );

Also, I'd prefer keeping the silent parameter because it can be useful in re-purposing a command class in multiple places. I think all the command-classes' run functions have this, for this purpose.

[sjinks]

I have updated dev-env-sync-sql.js to pass quiet: true in the constructor's parameters.

This is because, with VIP CLI Express, we won't be able to pass arbitrary arguments to run(). The correct approach will be to instantiate the command with the necessary parameters.

[aswasif007]

run function is being called from the dev-env-sync command with silent=true. Likely will break stuffs.

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[aswasif007]

Thanks for addressing the issue!