#@Erik Sipsma so I wrap `{ secret }` with

1 messages · Page 1 of 1 (latest)

acoustic temple
#

The way you configured the cache keys sounds reasonable to me.

I'm actually a little suspicious of this: https://github.com/dagger/dagger/blob/da4f730f32bd034a1f8525b645c6dd4de4080115/core/schema/address.go#L404-L407

You may be unconditionally always providing cacheKey arg there, even when it's empty string. Then the implementation of secret hits this line, sees that the cacheKey arg was set and uses it, even though it is actually just explicitly set to "".

I think it would be worth trying to only provide the cacheKey argument when it isn't ""

#

Or actually even simpler:

diff --git a/core/schema/secret.go b/core/schema/secret.go
index c6de6844c..ea21ddc8d 100644
--- a/core/schema/secret.go
+++ b/core/schema/secret.go
@@ -91,7 +91,7 @@ func (s *secretSchema) secret(
                return i, fmt.Errorf("failed to create instance: %w", err)
        }
 
-       if args.CacheKey.Valid {
+       if args.CacheKey.Valid && args.CacheKey.Value != "" {
                i = i.WithObjectDigest(dagql.HashFrom(string(args.CacheKey.Value)))
        } else {
                plaintext, err := secretStore.GetSecretPlaintextDirect(ctx, secret)

@sour locust I pulled your PR down and tested that diff and looks like it fixes the errors on TestCall/TestArgTypes/secret_args, you good with me pushing it to your PR?

acoustic temple
sour locust
#

Yeah the previous batch of errors was caused by logic to process local paths that made sense in the CLI, but didn't in the engine (ie. convert relative paths to absolute etc).

Maybe more of that kind of mistakes... I'll check 🙏

#

@acoustic temple looks like that particular error comes from:

    q := []dagql.Selector{
        {
            Field: "host",
        },
        {
            Field: "service",
            Args: []dagql.NamedInput{
                {
                    Name:  "host",
                    Value: dagql.Opt(dagql.NewString(host)),
                },
                {
                    Name:  "ports",
                    Value: ports,
                },
            },
        },
    }

is that dagql.Opt(dagql.NewString()) correct?

sour locust
#

(can't figure this one out...)