Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introducing a new approach to building Kopia commands using safecli/command package #2653

Merged
merged 11 commits into from
Mar 5, 2024
Merged

Introducing a new approach to building Kopia commands using safecli/command package #2653

merged 11 commits into from
Mar 5, 2024

Conversation

plar
Copy link
Contributor

@plar plar commented Feb 5, 2024 •
edited
Loading

Change Overview

This PR is the first in the series of PRs that will add a new way to build Kopia CLI commands.

The functionality introduced in this PR lays the foundation for subsequent PRs. It introduces a new version of safecli@v0.0.6, which features a higher level API. This API is designed to implement arguments, options, and options with arguments more effectively.

PR Train:

  1. PR-2653 Introducing a new approach to building Kopia CLI commands using safecli v0.0.6
  2. PR-2654 Implement Kopia common args and opts
  3. PR-2655 Implement Kopia storage helpers
  4. PR-2656 Add Kopia Filesystem storage opts
  5. PR-2657 Add Kopia GCS storage opts
  6. PR-2658 Add Kopia Azure storage opts
  7. PR-2659 Add Kopia S3 and S3 compliant storage opts
  8. PR-2661 Add Kopia repository create command
  9. PR-2662 Add Kopia repository connect command
  10. PR-2663 Add Kopia repository connect server command
  11. PR-2664 Add Kopia repository set-parameters command
  12. PR-2665 Add Kopia repository status command

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@plar plar self-assigned this Feb 5, 2024
@infraq infraq added this to In Progress in Kanister Feb 5, 2024
@plar plar changed the title Kopia cli core infra Implement Kopia core flag handling data structures and flag testing suite Feb 5, 2024
This was referenced Feb 6, 2024
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Merged
@plar plar requested a review from hairyhum February 12, 2024 20:13
ankitjain235
ankitjain235 reviewed Feb 13, 2024
View reviewed changes
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, added some minor questions/suggestions.

pkg/kopia/cli/internal/flag/string_flag.go Outdated Show resolved Hide resolved
pkg/kopia/cli/internal/flag/string_flag.go Outdated Show resolved Hide resolved
pkg/kopia/cli/internal/flag/flag_test.go Outdated Show resolved Hide resolved
@plar plar changed the title Implement Kopia core flag handling data structures and flag testing suite Introducing a new approach to building Kopia CLI commands using safecli v0.0.4 Feb 17, 2024
@plar plar changed the title Introducing a new approach to building Kopia CLI commands using safecli v0.0.4 Introducing a new approach to building Kopia commands using safecli/command package Feb 17, 2024
ankitjain235
ankitjain235 approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

LGTM, though I only see dependency import so I suppose only that's needed as part of this PR since functions have moved to safecli repo.

Kanister automation moved this from In Progress to Reviewer approved Feb 19, 2024
@plar
Add safecli dependency
1a3ee2d
@plar
add new flag implementations based on the safecli package for the Kop…
386e7e9 
…ia CLI
@plar
apply go fmt
b815f63 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
@plar
Fix Apply and test.Suit
bcafabb 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
@plar
pkg/kopia/cli/internal/flag is implemented in the safecli@v0.0.4 now
6d61bb4 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
@plar
Add pkg/kopia/cli package
e6ddb8f 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
@plar
go mod tidy
4e7ffd7 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
@plar
Update safecli to v0.0.5
f16aea7 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
@plar
Update safecli to v0.0.6
dcd6425 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
PrasadG193
PrasadG193 reviewed Feb 29, 2024
View reviewed changes
go.mod
Comment on lines +219 to +220
require github.com/kanisterio/safecli v0.0.6

Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] Can we merge this require block into any existing one instead of creating new?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would keep it in this PR since we have refs to this head PR everywhere. It looks like we have all PRs approved, so I'm going to merge this train on Monday.

pkg/kopia/cli/doc.go Outdated Show resolved Hide resolved
@plar
Fix formatting
44c133e 
Signed-off-by: pavel.larkin <pavel.larkin@veeam.com>
@plar plar added the kueue label Mar 5, 2024
@mergify mergify bot merged commit 2340f90 into master Mar 5, 2024
14 checks passed
Kanister automation moved this from Reviewer approved to Done Mar 5, 2024
@mergify mergify bot deleted the kopia-cli-core-infra branch March 5, 2024 17:33
@julio-lopez julio-lopez mentioned this pull request Jun 19, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hairyhum hairyhum hairyhum approved these changes

@PrasadG193 PrasadG193 PrasadG193 approved these changes

@ankitjain235 ankitjain235 ankitjain235 approved these changes

@e-sumin e-sumin e-sumin approved these changes

@Shrekster Shrekster Awaiting requested review from Shrekster

@pavannd1 pavannd1 Awaiting requested review from pavannd1

@kale-amruta kale-amruta Awaiting requested review from kale-amruta

Assignees

@plar plar

Labels
enhancement kueue
Projects
Kanister
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@plar @hairyhum @PrasadG193 @ankitjain235 @e-sumin