A question on modules + go/python differences....
Defined function in python:
#Module in Python + Go/Python SDK 'Consumers'
solid estuary
Using this function in a python sdk as a consumer:
Why does the go sdk consumer look like this?
choices has been wrapped in a go 'convention of 'opts' but only for the choices array.
If the python 'module' also declares a function which can also pass in an array of string value, go didn't do the same thing for a single arugment function:
so im just curious why it needed to do so for the one with mutliple arguments.
I think i figured it out, something to do with nil
sharp crow
definitely not the intended outcome based on a quick read
sharp crow
there's a known discrepancy between SDKs on how to interpret "optional" and default values
solid estuary
I think it might be due to None
sharp crow
@cobalt crater @graceful arrow does this ring a bell?
solid estuary
very interesting, because my choices is not None in the code
not defined to be None
Ah, but you said optional. Which is different. Okay yeah
graceful arrow
Yeah for optionals in python (dagger python sdk), the | None is required afaik
solid estuary
okay, but actually whats happening here is "pass in something, or a default will be used"
that doesnt feel like a None.
sharp crow
Yeah for optionals in python (dagger python sdk), the `| None` is required afaik
I think we may need to revisit that?
graceful arrow
In this case, an argument with a default is distinct from an optional argument. That is confusing
sharp crow
yeah there's an issue somewhere about harmonizing the definition across SDKs to:
- optional means default to null
- explicit default implies optional
I knew about this issue in Go but didn't realize we had it in python too
cobalt crater
<@768585883120173076> <@135620352201064448> does this ring a bell?
Yes, there's a fix, but I need to add backwards compat because it's a breaking change: https://github.com/dagger/dagger/pull/8106
solid estuary
I think we can get into some trickery with this kinda thing. I cant define those as None on Python side. IT causes issues with the module generating. Would be super useful for a page at some point that demonstrates from a hollistic view. Like a hello world in python, and then examples of using it in Go/ts, and visa versa. ... do it for all combinations and point out the differences. Very useful for potential module publishers
cobalt crater
Basically, Go doesn't consider a non-null with a default to be an optional argument. It equates non-null with required.
cobalt crater
Yeah for optionals in python (dagger python sdk), the `| None` is required afaik
Not true. Having a default makes an argument optional.
sharp crow
ah!
cobalt crater
It's just the Go codegen at a miss here, when generating the client bindings from the API.
sharp crow
here's where I left things: https://github.com/dagger/dagger/issues/6749#issuecomment-2078334724
do we still agree on this comment?
this is the recommendation that apparently we didn't get around to implementing right?
cobalt crater
this is the recommendation that apparently we didn't get around to implementing ...
Yes, I did. See #1283107857981509734 message, just above.
It's also on the last comment in that issue.
It was just holding on for v0.13 since it's a breaking change but then ran out of time to add the missing piece, i.e., backwards compat for older engines.
It was next on my list, just prioritized helping to drain the 0.13 milestone.
sharp crow
Right, thank you!
cobalt crater
May still get it in if release is tomorrow now.
sharp crow
(i meant back in April, we reached consensus but didn't go ahead and implement right away)
solid estuary
thanks for the replies. im learning a bit more each day too
cobalt crater
Well, it was a breaking change and it didn't end up getting prioritized for 0.12 so that leaves us 0.13.
cobalt crater
FYI, https://github.com/dagger/dagger/pull/8106 just got merged, so the fix will be released today.
