Skip to content

feat: Add on-conflict flag to influx-cli - #582

Open
devanbenz wants to merge 7 commits into
mainfrom
db/27578/restore-conflict
Open

feat: Add on-conflict flag to influx-cli #582
devanbenz wants to merge 7 commits into
mainfrom
db/27578/restore-conflict

Conversation

@devanbenz

Copy link
Copy Markdown
Contributor

Allows for user to indicate what to do during restore when there are buckets that are the same name as the ones being restored.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an --on-conflict option to influx restore so users can control how bucket name conflicts are handled during restore operations.

Changes:

  • Adds --on-conflict flag wiring to the influx restore command and restore.Params.
  • Introduces conflict-option parsing/serialization in the restore client and forwards the choice to the restore bucket-metadata API.
  • Extends the generated restore API request builder to include an onConflict query parameter.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File Description
cmd/influx/restore.go Adds the --on-conflict CLI flag and validates the provided value.
clients/restore/restore.go Defines conflict options, plumbs them into restore flows, and sends onConflict on restore bucket-metadata requests.
api/api_restore.gen.go Adds an optional onConflict query param to the generated PostRestoreBucketMetadata request.
Files not reviewed (1)
  • api/api_restore.gen.go: Generated file
Suppressed comments (1)

clients/restore/restore.go:373

  • restoreBucketLegacy accepts onConflict but currently never uses it. As a result, --on-conflict=skip|replace will have no effect against legacy (v2.0.x) servers, and restores will still fail on bucket name conflicts (or behave as the legacy API default). The legacy path should implement the same conflict behavior (skip/replace/error), e.g., by checking for an existing bucket by org+name and then either short-circuiting (skip) or deleting/recreating (replace).
func (c Client) restoreBucketLegacy(ctx context.Context, bkt br.ManifestBucketEntry, onConflict ConflictOption) (map[int64]int64, error) {
	log.Printf("INFO: Restoring bucket %q as %q using legacy APIs\n", bkt.BucketID, bkt.BucketName)
	// Legacy APIs require creating the bucket as a separate call.
	rps := make([]api.RetentionRule, len(bkt.RetentionPolicies))
	for i, rp := range bkt.RetentionPolicies {

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread clients/restore/restore.go Outdated
Comment thread clients/restore/restore.go
Comment thread clients/restore/restore.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • api/api_restore.gen.go: Generated file
Suppressed comments (2)

clients/restore/restore.go:320

  • Validation happens only after restoreOrg has already run. For callers that use Client.Restore directly, an invalid value can therefore create an organization before failing, and it is never rejected at all when no bucket matches. Parse params.OnConflict once before entering the bucket loop, then reuse the parsed option here.
		onConflict, err := ToConflictOption(params.OnConflict)
		if err != nil {
			return err

clients/restore/restore.go:371

  • This legacy implementation never uses onConflict. Since partialRestore routes v2.0.x servers here, --on-conflict skip and --on-conflict replace still attempt PostBuckets and fail when the bucket exists, silently violating the requested behavior. Implement the corresponding existing-bucket handling here, or explicitly reject unsupported modes before any restore side effects.
func (c Client) restoreBucketLegacy(ctx context.Context, bkt br.ManifestBucketEntry, onConflict ConflictOption) (map[int64]int64, error) {

@devanbenz devanbenz self-assigned this Aug 10, 2026
@devanbenz devanbenz added the enhancement New feature or request label Aug 10, 2026
@devanbenz
devanbenz marked this pull request as ready for review August 10, 2026 17:32
@devanbenz
devanbenz requested a review from gwossum August 10, 2026 17:32
Comment thread clients/restore/restore.go Outdated
Comment thread clients/restore/restore.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • api/api_restore.gen.go: Generated file
Suppressed comments (2)

clients/restore/restore.go:321

  • Validation occurs only after a manifest bucket matches and after restoreOrg may create an organization. Consequently, programmatic callers can receive success for an invalid option when no bucket matches, or mutate server state before receiving the validation error. Parse params.OnConflict once at the start of partialRestore, before the loop and any API calls, and reuse the parsed value.
		onConflict, err := ToConflictOption(params.OnConflict)
		if err != nil {
			return err
		}

clients/restore/restore.go:371

  • onConflict is never used in the legacy implementation, so against the supported v2.0.x server path both skip and replace still call PostBuckets and fail when the bucket already exists. Implement the three conflict modes for this path (or reject unsupported modes explicitly) instead of silently ignoring the flag.
func (c Client) restoreBucketLegacy(ctx context.Context, bkt br.ManifestBucketEntry, onConflict ConflictOption) (map[int64]int64, error) {

Comment thread cmd/influx/restore.go
},
&cli.StringFlag{
Name: "on-conflict",
Usage: "How to handle conflicting buckets -- i.e. buckets that already exist. Valid inputs: 'skip', 'replace', or 'error'.",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should state what the default is when the option is omitted. I thought maybe the CLI library would include this by default, but I just tried it and it does not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants