Go SDK: Discontinue +default pragma

[shykes]

I want to flag something in the new Go DX for optional arguments. I don't like encoding my default values in comments with +default, because they are no longer in my code. I can't use them if I need any value more trivial than a string. I can't use them in any way when calling functions intra-module. At this point I think we should remove +default and keep only +optional. It's not a one-way door: we can add it later. But if we keep it now, we will fragment how devs manage their default values.

[nipuna-perera]

I've been using +default a bunch and I like it. It works intra-module with the functionOpts pattern. What is the limitation you are running into? And if +default is removed what's the alternative?

[shykes]

I've been using +default a bunch and I like it. It works intra-module with the functionOpts pattern. What is the limitation you are running into? And if +default is removed what's the alternative?

What is the functionOpts pattern? Maybe that will help :)

[kpenfound]

My main issue when switching to the +optional pattern from Optional[] before discovering +default is that without +default there's no way to know if a value was default or user provided. For example, if I have a function that creates a container, and an optional cmd argument defaulted to "postgres", I have no way to know if an input of "" is a non-input, as in the user didn't provide a cmd arg, or they're explicitly inputting "". A // +default postgres solves this

[shykes]

My main issue when switching to the +optional pattern from Optional[] before discovering +default is that without +default there's no way to know if a value was default or user provided. For example, if I have a function that creates a container, and an optional cmd argument defaulted to "postgres", I have no way to know if an input of "" is a non-input, as in the user didn't provide a cmd arg, or they're explicitly inputting "". A // +default postgres solves this

Right, but for default values that can't be expressed in a +default comment, that problem still exists.

[nipuna-perera]

So when I call an existing module function from another module the +default values are passed through by default.

dag.One().MyFunction()

If I wanted to modify a default I can do

dag.One().MyFunction(MyFunctionOpts{ DefaultOne: "abc", DefaultTwo: "def" })

The idea that @vito also put forward is that for stuff that can't be set in +default (like core types) you can just expect it to be a nil and handle that within the function itself.

[shykes]

If I wanted to modify a default I can do

dag.One().MyFunction(MyFunctionOpts{ DefaultOne: "abc", DefaultTwo: "def" })

Is this something that works today? Or an idea for a future improvement?

The idea that @vito also put forward is that for stuff that can't be set in +default (like core types) you can just expect it to be a nil and handle that within the function itself.

Right, that's what I do today. But now I'm handling some of my defaults with +default, and some without. That's what I mean by a fragmented experience. The fragmentation itself is a problem.

[vito]

(on mobile so kinda terse, sorry)

+default also allows the default to show up in docs and in the schema which is really helpful imo. Would be sad to see it go even if intra-module calls are janky, because intra-module calls kinda suck for other reasons too (no delineation in TUI, no possibility for a cache hit when we add that). Would personally rather try to support self-calls better than remove this. I get that it only works for static values, but that's really common, and it's better than nothing.

[nipuna-perera]

Is this something that works today? Or an idea for a future improvement?

It works in 0.9.9

Right, that's what I do today. But now I'm handling some of my defaults with +default, and some without. That's what I mean by a fragmented experience. The fragmentation itself is a problem.

I agree with this. It's what I tried to bring up here :) - https://discord.com/channels/707636530424053791/1203102934901071882/1203151710218879016

IDK if removing it altogether is the best path though. I rather like being able to pass around defaults with the pragma. It's come in handy in a bunch of places.

[vito]

To try to clarify parts of the conversation:

• For inter-module calls (module A calling module B), +default works great - the value is in the schema itself, visible in Daggerverse, and can be overridden per-call via the SDK (Opts struct arg in Go, optional args in Python, etc). I believe that's what @nipuna-perera is referring to above.

• For intra-module calls (module A calling its own APIs internally) your only option right now is to call your own functions directly, which of course does not respect +default, so you end up having to check for zero-values or duplicate the default at each call site.

Previously we've discussed adding something like dag.Self() which would allow you to make self-calls through the API layer instead, which would respect +default and comes with other benefits of its own, but we haven't explored the complexity/tradeoffs of that approach in detail. There seems to be consensus that it's worth trying at least.

I'm only suggesting that the pain of implementing dag.Self() may be the lesser pain to to ripping out +default only to add it again later, since I don't think we're going to find a better way to specify defaults in Go any time soon. (Would love to be wrong, but this was one of our longer bikesheds...)

[shykes]

• For inter-module calls (module A calling module B), +default works great - the value is in the schema itself, visible in Daggerverse, and can be overridden per-call via the SDK (Opts struct arg in Go, optional args in Python, etc). I believe that's what @nipuna-perera is referring to above.

Some devs will still not like it, for arguable subjective reasons, because some logic that was previously in plain Go code, is now hidden in a black box controlled by a comment. We can still say "deal with it", but we should flag this as a possible clash with "native" preferences.

• For intra-module calls (module A calling its own APIs internally) your only option right now is to call your own functions directly, which of course does not respect +default, so you end up having to check for zero-values or duplicate the default at each call site.

Right.

Previously we've discussed adding something like dag.Self() which would allow you to make self-calls through the API layer instead, which would respect +default and comes with other benefits of its own, but we haven't explored the complexity/tradeoffs of that approach in detail. There seems to be consensus that it's worth trying at least.

Yes it would be great. Last time I heard about this, it had been tried, and abandoned because of a dependency cycle problem. It wasn't clear how likely it was that this could be solved. If it can, I'm all for it.

I'm only suggesting that the pain of implementing dag.Self() may be the lesser pain to to ripping out +default only to add it again later, since I don't think we're going to find a better way to specify defaults in Go any time soon. (Would love to be wrong, but this was one of our longer bikesheds...)

I'm fine with trying this. I guess at this point, my request is simply that we acknowledge the risk of Go developers not liking it for subjective reasons. Most of my actual problems would indeed be solved by a hypothetical dag.Self() in the future.

[shykes]

Follow-up. Could we perhaps require a valid go expression? So instead of +default=foo it would be +default="foo". That way, we leave the door open to +default=foo meaning "use the value of symbol foo", which would allow bridging into existing const values for example. It would also make it possible in the future to support more and more expressions. Also solves the problem of default values to integers etc.

Thoughts?

[vito]

I'm fine with trying this. I guess at this point, my request is simply that we acknowledge the risk of Go developers not liking it for subjective reasons. Most of my actual problems would indeed be solved by a hypothetical dag.Self() in the future.

Ack'd. I won't speak for others but my takeaway was that we had chosen the least bad solution, not a great one. We're trying to express a language feature that just doesn't exist in Go, and it doesn't seem like we can punt on it since that'd leave the Go SDK lesser than the rest (worse API docs), which seems like a bad precedent to set so early in the ecosystem.

Follow-up. Could we perhaps require a valid go expression? So instead of +default=foo it would be +default="foo"

I like this! Makes sense that this would work for const values. In the short-term, consts would at least let you share the same default until we figure out dag.Self(). I probably wouldn't want this forever though, since it feels like a lot of boilerplate.

Also solves the problem of default values to integers etc.

Integers actually work; for non-string values the value is assumed to be JSON. I found that a little surprising, so I'd be good with changing that to "foo" instead, which would free up foo to refer to a const.

[jedevc]

My main issue when switching to the +optional pattern from Optional[] before discovering +default is that without +default there's no way to know if a value was default or user provided. For example, if I have a function that creates a container, and an optional cmd argument defaulted to "postgres", I have no way to know if an input of "" is a non-input, as in the user didn't provide a cmd arg, or they're explicitly inputting "". A // +default postgres solves this

@kpenfound I think you can also do this with a pointer arg: e.g.

func (m *MyModule) DoThing(
	s *string, // +optional
) {

If s == nil, it wasn't set, if *s == "", the user set it to the empty string.

Most of my actual problems would indeed be solved by a hypothetical dag.Self() in the future.

Ack'd. I won't speak for others but my takeaway was that we had chosen the least bad solution, not a great one. We're trying to express a language feature that just doesn't exist in Go, and it doesn't seem like we can punt on it since that'd leave the Go SDK lesser than the rest (worse API docs), which seems like a bad precedent to set so early in the ecosystem.

Yup, I think we were aligned on this - we want dag.Self, it's just a lot of work which we haven't done yet. Some of the foundations were laid down in https://github.com/dagger/dagger/pull/6219.

Follow-up. Could we perhaps require a valid go expression? So instead of +default=foo it would be +default="foo"

I like this! Makes sense that this would work for const values. In the short-term, consts would at least let you share the same default until we figure out dag.Self(). I probably wouldn't want this forever though, since it feels like a lot of boilerplate.

Hm, I am a bit worried about this - having comments have this kind of ability to access variables, etc, does break a few expectations about how comments work. I get that we've already broken some stuff by having the pragmas in the first place, but this would be a step further. I do agree that we should potentially require quoting strings, so that we allow this in the future, but I'm not 100% sure we should allow the expression case right now (maybe we should split this out into a separate discussion).

+default also allows the default to show up in docs and in the schema which is really helpful imo. Would be sad to see it go even if intra-module calls are janky, because intra-module calls kinda suck for other reasons too (no delineation in TUI, no possibility for a cache hit when we add that).

Yup, this is the reason we have it all. When we were looking at doing defaults in the Go SDK there were a few ideas:

1. Don't do anything at all - this makes for bad docs, it means that the Go SDK just can't produce modules that fit into daggerverse in the same way that other languages do. While all languages are different, we should try and plaster over the biggest cracks so it's not a terrible experience for callers.

2. Try and encode this info into the type system, which is where Optional came from. The problem is, there's no way of encoding defaults into the type system easily. Ideally, we could use something like Rust's const-generics (and I expect the Rust SDK will do something like this), but those don't exist.

   The only other thing we managed to think of was to create a new type for each default value, and to define a Default method on it. This could work, but is actually hilariously verbose.

3. Do some insane magical introspection where we attempt to inspect the conditions that users do with the argument to "work out" a default value. I entertained this idea very briefly, but didn't really end up proposing a concrete idea - mostly because I'm pretty convinced this steps into the realm of actually computationally impossible.

4. Use comments to indicate this information - essentially the pragma solution we have today. It's kinda awful for intra-module calls (without dag.Self), but it does actually work. It also does have precedence in some large code-bases (though not end-user facing), such as the go internals, and the kubernetes project.

We picked the last one - mostly because:

• It's possible
• It lets us have docs + a good experience when browsing daggerverse
• It's not verbose

If Go SDK authors don't use // +default, I honestly think that's fine - the module they write will just be a bit less useful to browse in daggerverse. Until we can come up with another option that satisfies the above conditions, I think we have to keep // +default - I'd genuinely love another way of doing it, but I think with the limited nature of the go language, it's not really doable today.