Skip to content

Commit

Permalink
fix(cli): Don't return error when parameters unchanged (#3411)
Browse files Browse the repository at this point in the history
* don't return error when permissions unchanged
* remove logging since it only happens for a couple edge cases
  • Loading branch information
KastenMike committed Nov 1, 2023
1 parent bdc3314 commit fde0dfd
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
3 changes: 2 additions & 1 deletion cli/command_repository_set_parameters.go
Expand Up @@ -228,7 +228,8 @@ func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.Direc
requiredFeatures = c.addRemoveUpdateRequiredFeatures(requiredFeatures, &anyChange)

if !anyChange {
return errors.Errorf("no changes")
log(ctx).Info("no changes")
return nil
}

if blobcfg.IsRetentionEnabled() {
Expand Down
17 changes: 14 additions & 3 deletions cli/command_repository_set_parameters_test.go
Expand Up @@ -35,8 +35,10 @@ func (s *formatSpecificTestSuite) TestRepositorySetParameters(t *testing.T) {
require.Contains(t, out, "Max pack length: 21 MB")
require.Contains(t, out, fmt.Sprintf("Format version: %d", s.formatVersion))

_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters")
require.Contains(t, out, "no changes")

// failure cases
env.RunAndExpectFailure(t, "repository", "set-parameters")
env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=33")
env.RunAndExpectFailure(t, "repository", "set-parameters", "--max-pack-size-mb=9")
env.RunAndExpectFailure(t, "repository", "set-parameters", "--max-pack-size-mb=121")
Expand Down Expand Up @@ -73,6 +75,10 @@ func (s *formatSpecificTestSuite) TestRepositorySetParametersRetention(t *testin
_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters", "--retention-mode", "none")
require.Contains(t, out, "disabling blob retention")

// 2nd time also succeeds but disabling is skipped due to already being disabled. !anyChanges returns no error.
_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters", "--retention-mode", "none")
require.Contains(t, out, "no changes")

out = env.RunAndExpectSuccess(t, "repository", "status")
require.NotContains(t, out, "Blob retention mode")
require.NotContains(t, out, "Blob retention period")
Expand Down Expand Up @@ -201,21 +207,26 @@ func (s *formatSpecificTestSuite) TestRepositorySetParametersDowngrade(t *testin
require.Contains(t, out, "Format version: 1")
require.Contains(t, out, "Epoch Manager: disabled")
env.RunAndExpectFailure(t, "index", "epoch", "list")
// setting the current version again is ok
_, out = env.RunAndExpectSuccessWithErrOut(t, "repository", "set-parameters", "--index-version=1")
require.Contains(t, out, "no changes")
case format.FormatVersion2:
require.Contains(t, out, "Format version: 2")
require.Contains(t, out, "Epoch Manager: enabled")
env.RunAndExpectSuccess(t, "index", "epoch", "list")
_, out = env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=1")
require.Contains(t, out, "index format version can only be upgraded")
default:
require.Contains(t, out, "Format version: 3")
require.Contains(t, out, "Epoch Manager: enabled")
env.RunAndExpectSuccess(t, "index", "epoch", "list")
_, out = env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=1")
require.Contains(t, out, "index format version can only be upgraded")
}
}

checkStatusForVersion()

env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=1")

checkStatusForVersion()

// run basic check to ensure that an upgrade can still be performed as expected
Expand Down

0 comments on commit fde0dfd

Please sign in to comment.