#Schema views

1 messages · Page 1 of 1 (latest)

fair canyon
#

Continuing from #1254757484057464975 message.

@regal osprey, when do you use AllVersion vs AfterVersion for the new API? Is it whether there's differences in the dagql schema structure? The change in WithNewFile just removes the default value of an argument. I suppose implementation is like the one you've added for export?

regal osprey
#

So after version applies to every version after X, all version applies everywhere

#

It exists because if you want a field to have a view, it has to have a view on every declaration of that field (e.g. you can't declare it without a view and then with a view later)

#

The reason for this is subtle, it's enforced with panics, but it has to do with ensuring that the IDs for everything generate are somewhat reasonable

fair canyon
#

I'm testing it out with shellcheck and it's failing to load. Do I need docs generate or something?

regal osprey
#

Uh oh, hm, if you check the engine logs, are there any failures in there? You might be hitting one of the panics I mentioned

#

(I tried for so long to see if I could type check it with generics, it's feasible, but requires adding type parameters to almost every single struct in the engine)

fair canyon
# 
[buildkit] [trace=00000000000000000000000000000000] [client=7pl3l5dkslk97l9wbz24b4xfo] ------
[buildkit] [trace=00000000000000000000000000000000] [client=7pl3l5dkslk97l9wbz24b4xfo]  > exec go build -o /runtime .:
[buildkit] [trace=00000000000000000000000000000000] [client=7pl3l5dkslk97l9wbz24b4xfo] 0.441 # github.com/dagger/dagger/dev/shellcheck
[buildkit] [trace=00000000000000000000000000000000] [client=7pl3l5dkslk97l9wbz24b4xfo] 0.441 ./main.go:120:34: cannot use dagger.ContainerWithNewFileOpts{…} (value of type dagger.ContainerWithNewFileOpts) as string value in argument to base().WithExec([]string{…}).WithWorkdir("/src").WithNewFile
[buildkit] [trace=00000000000000000000000000000000] [client=7pl3l5dkslk97l9wbz24b4xfo] 0.441 ./main.go:121:4: unknown field Contents in struct literal of type dagger.ContainerWithNewFileOpts
[buildkit] [trace=00000000000000000000000000000000] [client=7pl3l5dkslk97l9wbz24b4xfo] ------
regal osprey
#

Hm okay, is this failing on a dagger develop?

fair canyon
#

dagger-dev call -m dev/shellcheck --help

regal osprey
#

Is Shell check using a v0.11.9 engineVersion as well?

#

Sorry I'm on mobile now, can't easily check, my bad

fair canyon
#

Yep, success with dagger v0.11.9. And with dagger-dev success if I update the code to use the contents as the required argument. Without any extra develop.

#

Going to share my changes.

regal osprey
#

Hmm okay, it sounds like the wrong views are getting applied

#

For whatever reason

fair canyon
regal osprey
#

I have an idea

#

If you remove the extend does it work? (And just duplicate the rest of the bits for now)

#

I suspect I might have screwed that up maybe

#

The views part looks perfectly fine to me

#

Tldr, extend tries to do some magic where it "pulls the field spec from the previous declaration"

#

So it's probably keeping the optional declaration

fair canyon
#

Yeah, that worked! 🙂

regal osprey
#

Awesome 🎉 I'll have a think and maybe Extend is just bad and should be removed

#

Or needs rework

#

It's very annoying to have to repeat all the doc strings, etc when there are no changes to those

fair canyon
#

That's pretty cool, now it works with both stable and dev 🎉 Good job!

regal osprey
#

🎉

fair canyon
#

With the schema duplicated, would you use AfterVersion or keep AllVersion for the new changes?

regal osprey
#

No strong opinions? It shouldn't affect it, but it's easy to change later

fair canyon
#

Yeah, I guess I prefer AllVersion to avoid hardcoding a specific version. And easier later when we decide to remove the compat (although I guess you'd remove the AllVersion too) 🙂

fair canyon
#

Hmm... there's an issue I didn't foresee. Runtime modules need to be stuck on an older version or avoid using apis with splitting views, otherwise they can't load modules on those older versions.

regal osprey
#

Wait hm why can't they load older versions? They should be able to right?

#

That potentially sounds like a bug, I thought they should be able to

#

(maybe there's something here with the follow-up pr I merged? I might have fixed something very similar in that one)

#

I can take a look at anything tomorrow fyi, I'll catch-up from everything

fair canyon
#

Maybe there's no issue 🤔 My thinking was that a user with an older cli/engine version can't run a module with a newer engineVersion. So I extrapolated that to mean that if a user is on v0.11.6 and is trying to load a Python module with "engineVersion": "0.11.6" it won't be able to if the Python runtime module is on v0.12. But, if the user is using cli/engine v0.11.6 then they will get the v0.11.6 bundled version of the Python runtime, not v0.12. Anything newer is fine.

The Python runtime module has a call to Container.withNewFile so I included in the PR to change it, but the engineVersion was set to "dev". I basically reverted to use the latest release, like in the dev module. That's when I realize there could be an incompatibility, but the SDK's version is tied to the cli/engine so I think we're good.

regal osprey
# fair canyon Maybe there's no issue 🤔 My thinking was that a user with an older cli/engine v...

My thinking was that a user with an older cli/engine version can't run a module with a newer engineVersion
technically this restriction doesn't exist yet - i suggested it in https://github.com/dagger/dagger/pull/7692#issuecomment-2191897660
i think we should probably have it 🎉

but the SDK's version is tied to the cli/engine so I think we're good.
yeahhhh, i think so too 😄 note, there is a bit of subtlety here, each module is always initialized with the dev version, so custom sdks don't benefit from this change - i have some idea for how to do this, but we really only need to do this when we modify the typedef api

fair canyon
regal osprey
#

it's a super edge case right now, so i don't really mind about it for this release

fair canyon
#

Oh, I see. You're talking about the SDK lib, not the SDK runtime module.

regal osprey
#

yup, sorry, my bad

fair canyon
#

@regal osprey, how do you suggest we do something like the schema views directly in codegen for https://github.com/dagger/dagger/pull/7773 ? No changes in schema in this case. Just pass on engineVersion and have each codegen parse and compare versions, or something else?

GitHub
💥 codegen: don't return `Void` by helderco · Pull Request #7773 · d...

WarningThis is a breaking change in Go! 💥

Fixes #6868
When depending on a module's function that doesn't return a value (returns only error or nothing):
func (m *SomeModule) DoSomething() ...

regal osprey
#

i suspect you might need a combination of them both?