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

Add --storage-id flag to Repo Create CLI #8557

Merged
merged 19 commits into from
Feb 10, 2025
Merged

Add --storage-id flag to Repo Create CLI #8557

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ad9e32c
Add --storage-id flag to Repo Create CLI
itaigilo Jan 27, 2025
94a246c
Remove flag documentation
itaigilo Feb 3, 2025
5411788
Add esti tests
itaigilo Feb 3, 2025
9a76eac
Fix tests
itaigilo Feb 4, 2025
c7ac2e2
Fix esti tests
itaigilo Feb 4, 2025
efe5832
Fix esti test
itaigilo Feb 4, 2025
44a2706
Fix tests
itaigilo Feb 4, 2025
c5bdb3c
Fix tests
itaigilo Feb 4, 2025
06aac0f
Update the tests
itaigilo Feb 5, 2025
71e445a
Merge branch 'master' into feature/add-storage-id-to-cli
itaigilo Feb 6, 2025
3700ede
Fix tests
itaigilo Feb 6, 2025
c6b4cbb
Update tests
itaigilo Feb 6, 2025
8b7c88d
Merge branch 'master' into feature/add-storage-id-to-cli
itaigilo Feb 6, 2025
2049918
Fix test file
itaigilo Feb 6, 2025
4159a45
Make flag hidden
itaigilo Feb 8, 2025
0e64d9e
Merge branch 'master' into feature/add-storage-id-to-cli
itaigilo Feb 8, 2025
9242822
Fix PR comments
itaigilo Feb 10, 2025
4b19558
Handle param error
itaigilo Feb 10, 2025
382207a
Use Must for flag
itaigilo Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion cmd/lakectl/cmd/repo_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const (

// repoCreateCmd represents the create repo command
var repoCreateCmd = &cobra.Command{
Use: "create <repository URI> <storage namespace>",
Use: "create <repository URI> <storage namespace> [--storage-id <storage id>]",
Copy link
Member

Choose a reason for hiding this comment

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

Lets not document this flag ATM and open an issue to discuss that

Short: "Create a new repository",
Example: "lakectl repo create " + myRepoExample + " " + myBucketExample,
Args: cobra.ExactArgs(repoCreateCmdArgs),
Expand All @@ -25,14 +25,18 @@ var repoCreateCmd = &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)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the difference between the Must and this error handling? What is the reason for this change?

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 think it's some merges mixed up.
Updating.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following a talk with @N-o-Z -
Changed it to Must. Same effect, also for optional params.

}
storageID, _ := cmd.Flags().GetString("storage-id")

resp, err := clt.CreateRepositoryWithResponse(cmd.Context(),
&apigen.CreateRepositoryParams{},
apigen.CreateRepositoryJSONRequestBody{
Name: u.Repository,
StorageId: &storageID,
StorageNamespace: args[1],
DefaultBranch: &defaultBranch,
})
Expand All @@ -48,6 +52,7 @@ var repoCreateCmd = &cobra.Command{
//nolint:gochecknoinits
func init() {
repoCreateCmd.Flags().StringP("default-branch", "d", DefaultBranch, "the default branch of this repository")
repoCreateCmd.Flags().String("storage-id", "", "the storage of this repository")
Copy link
Member

Choose a reason for hiding this comment

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

Suggest we make this a hidden flag for now


repoCmd.AddCommand(repoCreateCmd)
}
8 changes: 7 additions & 1 deletion cmd/lakectl/cmd/repo_create_bare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

// repoCreateBareCmd represents the create repo command
var repoCreateBareCmd = &cobra.Command{
Use: "create-bare <repository URI> <storage namespace>",
Use: "create-bare <repository URI> <storage namespace> [--storage-id <storage id>]",
Copy link
Member

Choose a reason for hiding this comment

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

Same comments as the "create" command

Short: "Create a new repository with no initial branch or commit",
Example: "lakectl create-bare " + myRepoExample + " " + myBucketExample,
Hidden: true,
Expand All @@ -20,16 +20,21 @@ 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)
}
storageID, _ := cmd.Flags().GetString("storage-id")

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)
Expand All @@ -44,6 +49,7 @@ 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().String("storage-id", "", "the storage of this repository")

repoCmd.AddCommand(repoCreateBareCmd)
}
6 changes: 4 additions & 2 deletions docs/reference/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2633,7 +2633,7 @@ Manage and explore repos
Create a new repository

```
lakectl repo create <repository URI> <storage namespace> [flags]
lakectl repo create <repository URI> <storage namespace> [--storage-id <storage id>] [flags]
```

#### Examples
Expand All @@ -2649,6 +2649,7 @@ lakectl repo create lakefs://my-repo s3://my-bucket
```
-d, --default-branch string the default branch of this repository (default "main")
-h, --help help for create
--storage-id string the storage of this repository
```


Expand Down Expand Up @@ -2731,7 +2732,7 @@ lakeFS plumbing command. Don't use unless you're _really_ sure you know what you
Create a new repository with no initial branch or commit

```
lakectl repo create-bare <repository URI> <storage namespace> [flags]
lakectl repo create-bare <repository URI> <storage namespace> [--storage-id <storage id>] [flags]
```

#### Examples
Expand All @@ -2747,6 +2748,7 @@ lakectl create-bare lakefs://my-repo s3://my-bucket
```
-d, --default-branch string the default branch name of this repository (will not be created) (default "main")
-h, --help help for create-bare
--storage-id string the storage of this repository
```


Expand Down
Loading