#0.18.11 enums breaking

1 messages ยท Page 1 of 1 (latest)

stoic sky
#

This is my enum

type Proximity string

const (
    // Local proxy also known as on-prem proxy.
    Local Proximity = "LOCAL"

    // AWS proxy should be always used within AWS.
    AWS Proximity = "AWS"
)

Publish

func (r *MyModule) Publish(
    ctx context.Context,
    user *dagger.Secret,
    pass *dagger.Secret,
    // Whether this is a local or cloud (AWS, Azure) Dagger engine
    // Options: LOCAL, AWS
    // +optional
    // +default="aws"
    proximity Proximity,
) ([]string, error) {
indigo whale
#

aha

#

"aws" should definitely be "AWS"

#

in the default

#

that said, yeah, this shouldn't be breaking

stoic sky
#

ooh!

indigo whale
#

the previous behavior definitely wasn't intentional lol

#

but i guess we should still support it

stoic sky
#

ok that fixed it. I had 3 different instances of mismatched cases.

#

And how it's working now makes sense. As long as you make it clear that it's breaking ๐Ÿ™‚

indigo whale
#

sigh ๐Ÿ˜ฆ welp thanks โค๏ธ
i guessed that we'd accidentally break something with this change ๐Ÿ˜„

stoic sky
#

Wait so does that mean I could have a "AWS" and a "aWs" and a "aws" as enums?

#

Just that I have to match the +default with the exact case..

indigo whale
#

๐Ÿค” one moment

#

dammit

indigo whale
#

so now, the enum NAME, is based on the variable name.

so if i have Foo Proximity = "bar"

#

then that exact enum pairing should appear in codegen (instead of previously Bar Promixity = "bar"

#

on the cli you pass the name - i.e. Foo (which matches how we do function or type naming)

stoic sky
#

Ah, and the +default should also match that (Foo)

indigo whale
#

well

#

for go specifically

#

we allow matching by name or value

stoic sky
#

gotcha, but it's case sensitive

indigo whale
#

in python or typescript we can be much more normal about it, since you can do arg=Proximity.Foo

#

but in go, we have this weird pragma-y thing

indigo whale
#

I can't quite work out why this used to work at all

stoic sky
#

yeah.. like in my case, what I had didn't match the case of either the name or the value

AWS Proximity = "AWS"

So that's confusing. I'm also confused why I didn't match it myself lol

indigo whale
#

i'll put together a regression fix

#

you won't be the only one ๐Ÿ˜„

stoic sky
#

for sure

indigo whale
#

thanks for upgrading so quickly and catching it ๐Ÿ˜„

stoic sky
#

Haha! I can't help living on the edge ๐Ÿ˜„ Do you want me to open an issue to track?

indigo whale
#

go for it ๐Ÿ™

#

actually!

#

might be worth holding

#

turns out. you can't actually call that function and rely on the default value

#

even in 0.18.10

#

the module "works" - until you try and call it

#

e.g.

const (
    // Local proxy also known as on-prem proxy.
    Local Proximity = "LOCAL"

    // AWS proxy should be always used within AWS.
    AWS Proximity = "AWS"
)

// Returns a container that echoes whatever string argument is provided
func (m *Test) Test(
    // +default="aws"
    p Proximity,
) *dagger.Container {
    return dag.Container().From("alpine:latest").WithExec([]string{"echo", string(p)})
}
#
โฏ dagger call test
โฏ DAGGER_LEAVE_OLD_ENGINE=1 dagger-v0.18.10 call test
โ–ถ connect 0.2s
โ–ถ load module: . 1.0s
โ— parsing command line arguments 0.0s

โ— test: Test! 0.0s
โœ˜ .test: Container! 0.2s ERROR
! failed to set call inputs: failed to convert arg "p": invalid enum value "aws"
stoic sky
#

oh? but I do call it

indigo whale
#

if you fill in the default it'll work

#

but if you leave it blank to rely on the default it'll fail

stoic sky
#

gotcha!

#

Yes I provide it every time

indigo whale
#

we should amend the release notes - but i think ensuring that the default values are validated is fairly reasonable

stoic sky
#

Yes I agree.

indigo whale
#

we don't need bug for bug compatibility ๐Ÿ˜ข

stoic sky
#

Hm, does this break compat though? I updated the +default but kept the module engine version 0.18.9 and updated the CLI/engine version to 0.18.11 and that threw a compiler error for dagger.gen.go

indigo whale
#

yes this would break compat

stoic sky
#

good call out I guess for notes

indigo whale
stoic sky
#

hm, does breaking compat mean it breaks the whole chain? Say I have module A and B

A is dagger 0.18.9
B is dagger 0.18.11

If A has an enum and B calls A, is that supposed to break?

#

I see this when I try to run. I actually don't see it when I do dagger develop (ModuleAType is made up to mask real type name)

10:58:24  internal/dagger/dagger.gen.go:11779:8: ModuleAType (type) is not an expression
10:58:24  internal/dagger/dagger.gen.go:11798:2: ModuleAType redeclared in this block
10:58:24      internal/dagger/dagger.gen.go:11747:6: other declaration of ModuleAType
indigo whale
#

hm, this one looks different

#

do you have the definition of ModuleAType?

#

we do have a test to check this exact scenario, i suspect something a little weird is happening

#

(to clarify, sounds like a real regression here)

stoic sky
#

oh? When I upgraded ModuleA to 0.18.11 the issue went away though. Let me get you the def

#

It looks a lot like the other one

// ProxyType is the type of proxy to be used for the container.
type ProxyType string

const (
    // Default value, no proxy.
    None ProxyType = "NONE"

    // Local proxy also known as on-prem proxy.
    Local ProxyType = "LOCAL"

    // AWS proxy should be always used within AWS.
    AWS ProxyType = "AWS"
)
#

So, are you saying my ModuleA can remain 0.18.9 while B is 0.18.11?

indigo whale
#

rip, okay, lol

#

so this is actually because AWS name and value matches

#

and this breaks

indigo whale
indigo whale
stoic sky
#

Ooh! I'm glad that's the case. Is this a hotfix worthy fix? Or should I hold my upgrade to 0.18.11?

indigo whale
stoic sky
#

I'm going to hold off on updating and maybe even skip 0.18.11

indigo whale
#

sounds good ๐Ÿ˜… we should be shipping a v0.18.12 before the end of week, have spent all day trying to get fixes for this prepped

stoic sky
#

You rock! Thank you for the quick follow up.

stoic sky
#

Happy to report v0.18.12 fixed all the enum issues I had. Thank you for the quick turnaround!! โค๏ธ

half portal
#

@stoic sky I'm still getting this unknown enum value error in 0.18.12 - what else did you change to resolve this?

stoic sky
#

What does your enum def look like?

half portal
#
DV Environment = "dv"
AT Environment = "at"
PR Environment = "pr"
    // Environment code
    environment Environment,
indigo whale
#

are you storing the enum value in a struct?

half portal
#

No, just passing it to a few functions from the function it's an argument of

stoic sky
#

DV Environment = "dv"
Try DV Environment = "DV"?

half portal
#

Same error

indigo whale
#

okay, how are you using the enum? i can't repro with the enum defs you've shared

#
dagger call do --env DV
โ–ถ connect 0.2s
โ–ถ load module: . 4.3s
โ— parsing command line arguments 0.0s

โ— test: Test! 0.0s
โ–ถ .do(env: "DV"): String! 0.4s

dv
half portal
#

It's passed to a function which accepts a parameter of type Environment

indigo whale
#
package main

type Environment string

var (
    DV Environment = "dv"
    AT Environment = "at"
    PR Environment = "pr"
)

type Test struct{}

// Returns a container that echoes whatever string argument is provided
func (m *Test) Do(env Environment) Environment {
    return DV
}
half portal
#

m.python(..., ..., ..., ..., environment, ...)

func (m *ServiceDeployer) python(
    ...
    environment Environment,
    ...
)
indigo whale
#

what does dagger core version say

#

and dagger version

#

ah

#

you need to use const not var

#

i'm quite sure how you're even seeing an enum anywhere, dagger isn't detecting that

half portal
#

dagger core version v0.18.12

#

dagger version v0.18.12

half portal
#

Moving it to var doesn't work, same error

#

There's something else going on here @indigo whale, if I replace that enum with strings and update the functions that accept that as a parameter it still errors on unknown enum value

indigo whale
#

are there any other enums?

half portal
#

Yes