From 502432dd2f339f805e42395667784ed476be599b Mon Sep 17 00:00:00 2001 From: itaigilo Date: Mon, 10 Feb 2025 19:53:42 +0200 Subject: [PATCH] Add --storage-id flag to Repo Create CLI (#8557) --- cmd/lakectl/cmd/repo_create.go | 18 +++++++++++------- cmd/lakectl/cmd/repo_create_bare.go | 13 ++++++++----- cmd/lakectl/cmd/root.go | 8 ++++++++ .../lakectl_repo_create_with_storage_id.golden | 3 +++ esti/lakectl_test.go | 13 +++++++++++++ 5 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 esti/golden/lakectl_repo_create_with_storage_id.golden diff --git a/cmd/lakectl/cmd/repo_create.go b/cmd/lakectl/cmd/repo_create.go index ad7b859407c..135096ff97e 100644 --- a/cmd/lakectl/cmd/repo_create.go +++ b/cmd/lakectl/cmd/repo_create.go @@ -9,9 +9,9 @@ import ( ) const ( - DefaultBranch = "main" - - SampleDataFlag = "sample-data" + defaultBranchFlagName = "default-branch" + defaultBranchFlagValue = "main" + sampleDataFlagName = "sample-data" repoCreateCmdArgs = 2 ) @@ -27,13 +27,16 @@ var repoCreateCmd = &cobra.Command{ clt := getClient() u := MustParseRepoURI("repository URI", args[0]) fmt.Println("Repository:", u) - defaultBranch := Must(cmd.Flags().GetString("default-branch")) - sampleData := Must(cmd.Flags().GetBool(SampleDataFlag)) + + defaultBranch := Must(cmd.Flags().GetString(defaultBranchFlagName)) + sampleData := Must(cmd.Flags().GetBool(sampleDataFlagName)) + storageID := Must(cmd.Flags().GetString(storageIDFlagName)) resp, err := clt.CreateRepositoryWithResponse(cmd.Context(), &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{ Name: u.Repository, + StorageId: &storageID, StorageNamespace: args[1], DefaultBranch: &defaultBranch, SampleData: &sampleData, @@ -53,8 +56,9 @@ var repoCreateCmd = &cobra.Command{ //nolint:gochecknoinits func init() { - repoCreateCmd.Flags().StringP("default-branch", "d", DefaultBranch, "the default branch of this repository") - repoCreateCmd.Flags().Bool(SampleDataFlag, false, "create sample data in the repository") + repoCreateCmd.Flags().StringP(defaultBranchFlagName, "d", defaultBranchFlagValue, "the default branch of this repository") + repoCreateCmd.Flags().Bool(sampleDataFlagName, false, "create sample data in the repository") + withStorageID(repoCreateCmd) repoCmd.AddCommand(repoCreateCmd) } diff --git a/cmd/lakectl/cmd/repo_create_bare.go b/cmd/lakectl/cmd/repo_create_bare.go index 99dc2d9ef08..d636744b10b 100644 --- a/cmd/lakectl/cmd/repo_create_bare.go +++ b/cmd/lakectl/cmd/repo_create_bare.go @@ -20,16 +20,18 @@ var repoCreateBareCmd = &cobra.Command{ clt := getClient() u := MustParseRepoURI("repository URI", args[0]) fmt.Println("Repository:", u) - defaultBranch, err := cmd.Flags().GetString("default-branch") - if err != nil { - DieErr(err) - } + + defaultBranch := Must(cmd.Flags().GetString(defaultBranchFlagName)) + storageID := Must(cmd.Flags().GetString(storageIDFlagName)) + bareRepo := true + resp, err := clt.CreateRepositoryWithResponse(cmd.Context(), &apigen.CreateRepositoryParams{ Bare: &bareRepo, }, apigen.CreateRepositoryJSONRequestBody{ DefaultBranch: &defaultBranch, Name: u.Repository, + StorageId: &storageID, StorageNamespace: args[1], }) DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated) @@ -43,7 +45,8 @@ var repoCreateBareCmd = &cobra.Command{ //nolint:gochecknoinits func init() { - repoCreateBareCmd.Flags().StringP("default-branch", "d", DefaultBranch, "the default branch name of this repository (will not be created)") + repoCreateBareCmd.Flags().StringP(defaultBranchFlagName, "d", defaultBranchFlagValue, "the default branch name of this repository (will not be created)") + withStorageID(repoCreateBareCmd) repoCmd.AddCommand(repoCreateBareCmd) } diff --git a/cmd/lakectl/cmd/root.go b/cmd/lakectl/cmd/root.go index 2a18cf6da12..ea51b1c34aa 100644 --- a/cmd/lakectl/cmd/root.go +++ b/cmd/lakectl/cmd/root.go @@ -158,6 +158,7 @@ var ( const ( recursiveFlagName = "recursive" recursiveFlagShort = "r" + storageIDFlagName = "storage-id" presignFlagName = "pre-sign" parallelismFlagName = "parallelism" noProgressBarFlagName = "no-progress" @@ -187,6 +188,13 @@ func withRecursiveFlag(cmd *cobra.Command, usage string) { cmd.Flags().BoolP(recursiveFlagName, recursiveFlagShort, false, usage) } +func withStorageID(cmd *cobra.Command) { + cmd.Flags().String(storageIDFlagName, "", "") + if err := cmd.Flags().MarkHidden(storageIDFlagName); err != nil { + DieErr(err) + } +} + func withParallelismFlag(cmd *cobra.Command) { cmd.Flags().IntP(parallelismFlagName, "p", defaultParallelism, "Max concurrent operations to perform") diff --git a/esti/golden/lakectl_repo_create_with_storage_id.golden b/esti/golden/lakectl_repo_create_with_storage_id.golden new file mode 100644 index 00000000000..8673945308c --- /dev/null +++ b/esti/golden/lakectl_repo_create_with_storage_id.golden @@ -0,0 +1,3 @@ +Repository: lakefs://${REPO} +storage id: invalid value: validation error +400 Bad Request diff --git a/esti/lakectl_test.go b/esti/lakectl_test.go index 2d3f67e90c1..2048dd9bdcc 100644 --- a/esti/lakectl_test.go +++ b/esti/lakectl_test.go @@ -113,6 +113,19 @@ func TestLakectlBasicRepoActions(t *testing.T) { RunCmdAndVerifySuccessWithFile(t, Lakectl()+" repo create lakefs://"+repoName3+" "+storage3+" --sample-data", false, "lakectl_repo_create_sample", vars) } +func TestLakectlRepoCreateWithStorageID(t *testing.T) { + // Validate the --storage-id flag (currently only allowed to be empty) + repoName := generateUniqueRepositoryName() + storage := generateUniqueStorageNamespace(repoName) + vars := map[string]string{ + "REPO": repoName, + "STORAGE": storage, + "BRANCH": mainBranch, + } + RunCmdAndVerifyFailureWithFile(t, Lakectl()+" repo create lakefs://"+repoName+" "+storage+" --storage-id storage1", false, "lakectl_repo_create_with_storage_id", vars) + RunCmdAndVerifySuccessWithFile(t, Lakectl()+" repo create lakefs://"+repoName+" "+storage+" --storage-id \"\"", false, "lakectl_repo_create", vars) +} + func TestLakectlPreSignUpload(t *testing.T) { repoName := generateUniqueRepositoryName() storage := generateUniqueStorageNamespace(repoName)