-
Notifications
You must be signed in to change notification settings - Fork 561
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.
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
feat: supportAllowWildCard
and AllowNotFound
#7437
base: main
Are you sure you want to change the base?
Changes from all commits
aaf55e4
49a1117
88d6233
5adba3b
7c80e4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
package main | ||
|
||
import ( | ||
"context" | ||
|
||
"golang.org/x/sync/errgroup" | ||
) | ||
|
||
type AllSDK struct { | ||
SDK *SDK // +private | ||
} | ||
|
||
var _ sdkBase = AllSDK{} | ||
|
||
func (t AllSDK) Lint(ctx context.Context) error { | ||
eg, ctx := errgroup.WithContext(ctx) | ||
for _, sdk := range t.SDK.allSDKs() { | ||
sdk := sdk | ||
eg.Go(func() error { | ||
return sdk.Lint(ctx) | ||
}) | ||
} | ||
return eg.Wait() | ||
} | ||
|
||
func (t AllSDK) Test(ctx context.Context) error { | ||
eg, ctx := errgroup.WithContext(ctx) | ||
for _, sdk := range t.SDK.allSDKs() { | ||
sdk := sdk | ||
eg.Go(func() error { | ||
return sdk.Test(ctx) | ||
}) | ||
} | ||
return eg.Wait() | ||
} | ||
|
||
func (t AllSDK) Generate(ctx context.Context) (*Directory, error) { | ||
eg, ctx := errgroup.WithContext(ctx) | ||
dirs := make([]*Directory, len(t.SDK.allSDKs())) | ||
for i, sdk := range t.SDK.allSDKs() { | ||
i, sdk := i, sdk | ||
eg.Go(func() error { | ||
dir, err := sdk.Generate(ctx) | ||
if err != nil { | ||
return err | ||
} | ||
dir, err = dir.Sync(ctx) | ||
if err != nil { | ||
return err | ||
} | ||
dirs[i] = dir | ||
return nil | ||
}) | ||
} | ||
err := eg.Wait() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
dir := dag.Directory() | ||
for _, dir2 := range dirs { | ||
dir = dir.WithDirectory("", dir2) | ||
} | ||
return dir, nil | ||
} | ||
|
||
func (t AllSDK) Bump(ctx context.Context, version string) (*Directory, error) { | ||
eg, ctx := errgroup.WithContext(ctx) | ||
dirs := make([]*Directory, len(t.SDK.allSDKs())) | ||
for i, sdk := range t.SDK.allSDKs() { | ||
i, sdk := i, sdk | ||
eg.Go(func() error { | ||
dir, err := sdk.Bump(ctx, version) | ||
if err != nil { | ||
return err | ||
} | ||
dir, err = dir.Sync(ctx) | ||
if err != nil { | ||
return err | ||
} | ||
dirs[i] = dir | ||
return nil | ||
}) | ||
} | ||
err := eg.Wait() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
dir := dag.Directory() | ||
for _, dir2 := range dirs { | ||
dir = dir.WithDirectory("", dir2) | ||
} | ||
return dir, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -521,6 +521,13 @@ | |
require.NoError(t, err) | ||
require.Equal(t, []string{"some-dir", "some-file"}, entries) | ||
|
||
// Test without dir with allow not found = false | ||
_, err = dir1. | ||
WithoutDirectory("non-existant", dagger.DirectoryWithoutDirectoryOpts{AllowNotFound: false}). | ||
Check failure on line 526 in core/integration/directory_test.go
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TomChv this is your issue. In go, the empty struct defaults to What you need is to invert the option, to something like |
||
Entries(ctx) | ||
|
||
require.Error(t, err) | ||
|
||
dir := c.Directory(). | ||
WithNewFile("foo.txt", "foo"). | ||
WithNewFile("a/bar.txt", "bar"). | ||
|
@@ -550,6 +557,12 @@ | |
require.NoError(t, err) | ||
require.Equal(t, []string{"data.json"}, entries) | ||
|
||
_, err = dir. | ||
WithoutDirectory("b/*.txt", dagger.DirectoryWithoutDirectoryOpts{AllowWildCard: false}). | ||
Entries(ctx) | ||
|
||
require.Error(t, err) | ||
|
||
entries, err = dir. | ||
WithoutFile("c/*a1*"). | ||
Entries(ctx, dagger.DirectoryEntriesOpts{Path: "c"}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,9 @@ func (s *directorySchema) Install() { | |
ArgDoc("permissions", `Permission given to the copied file (e.g., 0600).`), | ||
dagql.Func("withoutFile", s.withoutFile). | ||
Doc(`Retrieves this directory with the file at the given path removed.`). | ||
ArgDoc("path", `Location of the file to remove (e.g., "/file.txt").`), | ||
ArgDoc("path", `Location of the file to remove (e.g., "/file.txt").`). | ||
ArgDoc("allowWildCard", `Allow wildcards in the path (e.g., "*.txt").`). | ||
ArgDoc("allowNotFound", `Allow the operation to not fail if the directory does not exist.`), | ||
Comment on lines
+58
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI - we would also want this on |
||
dagql.Func("directory", s.subdirectory). | ||
Doc(`Retrieves a directory at the given path.`). | ||
ArgDoc("path", `Location of the directory to retrieve (e.g., "/src").`), | ||
|
@@ -71,7 +73,9 @@ func (s *directorySchema) Install() { | |
ArgDoc("permissions", `Permission granted to the created directory (e.g., 0777).`), | ||
dagql.Func("withoutDirectory", s.withoutDirectory). | ||
Doc(`Retrieves this directory with the directory at the given path removed.`). | ||
ArgDoc("path", `Location of the directory to remove (e.g., ".github/").`), | ||
ArgDoc("path", `Location of the directory to remove (e.g., ".github/").`). | ||
ArgDoc("allowWildCard", `Allow wildcards in the path (e.g., "*.txt").`). | ||
ArgDoc("allowNotFound", `Allow the operation to not fail if the directory does not exist.`), | ||
dagql.Func("diff", s.diff). | ||
Doc(`Gets the difference between this directory and an another directory.`). | ||
ArgDoc("other", `Identifier of the directory to compare.`), | ||
|
@@ -233,18 +237,22 @@ func (s *directorySchema) withFiles(ctx context.Context, parent *core.Directory, | |
|
||
type withoutDirectoryArgs struct { | ||
Path string | ||
AllowWildCard bool `default:"true"` | ||
AllowNotFound bool `default:"true"` | ||
} | ||
|
||
func (s *directorySchema) withoutDirectory(ctx context.Context, parent *core.Directory, args withoutDirectoryArgs) (*core.Directory, error) { | ||
return parent.Without(ctx, args.Path) | ||
return parent.Without(ctx, args.Path, args.AllowWildCard, args.AllowNotFound) | ||
} | ||
|
||
type withoutFileArgs struct { | ||
Path string | ||
AllowWildCard bool `default:"true"` | ||
AllowNotFound bool `default:"true"` | ||
} | ||
|
||
func (s *directorySchema) withoutFile(ctx context.Context, parent *core.Directory, args withoutFileArgs) (*core.Directory, error) { | ||
return parent.Without(ctx, args.Path) | ||
return parent.Without(ctx, args.Path, args.AllowWildCard, args.AllowNotFound) | ||
} | ||
|
||
type diffArgs struct { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. I didn't know we defaulted to this.
This feels slightly inconsistent with other stuff - see another discussion on magic processing in #7173. A bit strange that some paths can be globs/wildcards/expansions and some aren't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes definitely, I think that's why we wanted to add the parameter.