GraphQL casing

[astral shoal]

@oblique furnace, where does the server handle casing for the SDK? I'm sending an arg as snake_case, but getting input as camelCase.

[oblique furnace]

Oh right, I might have missed handling that for args (did it for objects+funcs+fields). The args are set here: https://github.com/sipsma/dagger/blob/9130e1f0b64c0fa51b2576797f3ec58102f320c5/core/schema/module.go#L371-L371

I updated Function here to store the "original" name: https://github.com/sipsma/dagger/blob/9130e1f0b64c0fa51b2576797f3ec58102f320c5/core/function.go#L26-L26

Think we probably just need the same thing on FunctionArg here: https://github.com/sipsma/dagger/blob/9130e1f0b64c0fa51b2576797f3ec58102f320c5/core/function.go#L78-L78

[astral shoal]

Do you have bandwidth to fix that? I started doing it but it's late for me.

[oblique furnace]

Yes, may not make it to the release today but I will fix tonight if nothing else

[astral shoal]

Will we make another release this week to get python classes in?

[oblique furnace]

Yes for sure

We just have a lot of bug fixes built up that I'd prefer to get out today

[oblique furnace]

@astral shoal I haven't checked the Python PR yet, do you think it's a pretty straightforward merge?

If so, maybe we can just get it merged in the next few hours and have part of the release

[astral shoal]

Yes, but I need the args casing.

I'm looking at functionResolver trying to figure where would I pick up an OriginalName from FunctionArg

[oblique furnace]

Looking at this now while I wait for CI to run on other stuff

I'll just pull your PR @astral shoal and attempt the fix on top of that

[astral shoal]

I fixed it.

Not sure if it's the right way:

[oblique furnace]

Okay well thank you for fixing it and sorry I took too long for me to reach this in the queue 😅

[oblique furnace]

That looks like essentially what I just started doing

Okay I'll just start reviewing your PR then

Provided nothing blocking should be able to get it in the release

[astral shoal]

Tests are passing, I'm going to rebase.

[oblique furnace]

I hit an error when trying to use container as a field: https://github.com/dagger/dagger/pull/6016#issuecomment-1786081176

Doesn't need to block but if it's a real bug and not just me messing up, then would be good to fix before merge. otherwise lgtm!

[astral shoal]

I def need to add more tests, especially around dagger types, and other custom types not decorated. Looks like a bug as the example looks valid. I'll investigate.

[oblique furnace]

Okay sounds good, if it's too much to fix right now I think we can merge as is and just fix in follow-ups

[astral shoal]

Yeah, if we're up the wall, but give me a few minutes if you can spare.

[oblique furnace]

Yes no rush! Doesn't matter if the release is in a few hours; I'll be doing it so it's like we're forcing Gerhard to stay up too late 🙂

[astral shoal]

I believe I found the issue.

[astral shoal]

There's actually no bug. I made an integration test on the serialization part to guard against it, just need to maybe give a better error message.

[oblique furnace]

Oh okay, so what is the user source code that will result in it being successfully called? I tried making bar async but I got the same thing

[astral shoal]

You need async in the function and await on stdout.

Because stdout() is a coroutine function.

[oblique furnace]

I see! Cool yeah better error messages are needed all around, so not a blocker

oh nevermind you already pushed that 🙂

[astral shoal]

I checked out the previous commit and can't reproduce now. I must be getting a cached sdk, even after hack/dev.

With my await_maybe, it's possible the sdk does the await for you if you forget. Not sure if it's a good thing though.

[oblique furnace]

I just tried again with your latest changes pulled down and am getting the same error as before,

must be getting a cached sdk, even after hack/dev
There's some problems rn where we need to invalidate cache when the engine changes; in meantime you can do docker rm -fv dagger-engine.dev; docker volume rm dagger-engine.dev to completely clear cache

import dagger
from dagger.mod import Annotated, Doc, field, function, object_type


@object_type
class Thing:
    """Blahblahblah"""

    ctr: Annotated[
        dagger.Container,
        Doc("something about a container"),
    ] = field()

    @function
    async def bar(self) -> str:
        return await self.ctr.with_exec(["cat", "/etc/os-release"]).stdout()

@function
def foo() -> Thing:
    return Thing(ctr=dagger.container().from_("alpine:latest"))

I'm still getting the error from before w/ that ^

(I know it's late for you, we can just call it here and merge as is if you need to go)

[astral shoal]

Try removing the annotation.

Just do ctr: dagger.Container = field().

[oblique furnace]

It worked 🎉

Okay, that's good enough for me, unless you want to push anything else

[astral shoal]

Ok for now, I'll fix the annotation tomorrow. Must be cattrs not knowing how to deal with Annotated

[oblique furnace]

Cool approved, just need the stupid flaky testdev workflow to pass eventually now and will merge it.

If you have one more sec, I have a PR out that improves that test: https://github.com/dagger/dagger/pull/6018 May honestly be faster to merge that and then rebase your PR (which I am happy to do)

If you gotta go though then no worries, I can take it from here

[astral shoal]

Yep, np 🙂

Rebased mine.

[oblique furnace]

Thanks!

[oblique furnace]

Merged!

[astral shoal]

I think the issue is this: https://github.com/python-attrs/cattrs/issues/418

That's the version we have. So it should be fixed in the next version.