Page MenuHomePhabricator

`storage --dry-run` actually looks rather wet when applying patches
Closed, ResolvedPublic

Description

The refactoring in D16912 looks to have made storage --dry-run actually be wet. Namely:

if ($is_dryrun) {
  echo tsprintf(
    "%s\n",
    pht(
      'DRYRUN: Would apply patch "%s" to host "%s".',
      $key,
      $ref_key));
} else {
  echo tsprintf(
    "%s\n",
    pht(
      'Applying patch "%s" to host "%s"...',
      $key,
      $ref_key));
}

$t_begin = microtime(true);
$api->applyPatch($patch);
$t_end = microtime(true);

...looks to apply the patch regardless of the --dry-run flag. That is certainly what we just observed in production, as well.

Running rP 2017 week 17.

Revisions and Commits

Restricted Differential Revision
Restricted Differential Revision

Event Timeline

guess macro has a bug too

epriestley added a subscriber: epriestley.

Possibly we should just remove this flag because bin/storage status is equivalent and bin/storage adjust prompts by default, buuut...

epriestley added a revision: Restricted Differential Revision.May 6 2017, 2:56 AM
epriestley closed this task as Resolved by committing Restricted Diffusion Commit.May 6 2017, 2:57 AM
epriestley added a commit: Restricted Diffusion Commit.

Sorry about that! Thanks for the report.

epriestley added a commit: Restricted Diffusion Commit.May 6 2017, 2:58 AM