#Shell outside module

1 messages · Page 1 of 1 (latest)

halcyon eagle
#

Let's continue here 🙂

dusk frigate
#

on v0.15.4, if there was no module, it would do this:

❯ pwd
/home/jedevc
❯ dagger-v0.15.4 shell
Dagger interactive shell. Type ".help" for more information. Press Ctrl+D to exit.
⋈
halcyon eagle
#

Which branch are you using?

dusk frigate
#

on v0.16:

❯ dagger-v0.16.1 shell
✔ connect 0.2s
│ ✔ starting engine 0.0s
│ │ ✔ create 0.0s
│ │ │ ✔ exec docker ps -a --no-trunc --filter name=^/dagger-engine- --format {{.Names}} 0.0s
│ │ │ ┃ dagger-engine-v0.15.4                                                                                                                                                     
│ │ │ ┃ dagger-engine-v0.16.1                                                                                                                                                     
│ │ │ ✔ exec docker start dagger-engine-v0.16.1 0.0s
│ │ │ ┃ dagger-engine-v0.16.1                                                                                                                                                     
│ 
│ ✔ connecting to engine 0.0s
│ │ ✔ moby.buildkit.v1.Control/Info 0.0s
│ │ ✔ moby.buildkit.v1.Control/Info 0.0s
│ 
│ ✔ starting session 0.1s
│ ✔ subscribing to telemetry 0.0s
│ │ ✔ consuming /v1/traces 0.0s
│ │ ✔ consuming /v1/logs 0.0s
│ │ ✔ consuming /v1/metrics 0.0s

✘ load module 2.1s
! failed to get configured module: Post "http://dagger/query": context canceled
│ ✘ finding module configuration 2.1s
│ ! Post "http://dagger/query": context canceled
│ │ ● moduleSource(refString: "."): ModuleSource! 2.1s
│ │ │ ✔ parseRefString: . 0.0s
│ │ │ 
│ │ │ ✔ host: Host! 0.0s
│ │ │ ● .directory(path: "/home/jedevc"): Directory! 2.1s
│ │ │ │ ● upload /home/jedevc from b32t5hc5ezsulbudioavkyjil (client id: jjyd4o4c3ggeooqbsvq4i8cw7, session id: o08sruew63eklmkooxebeo3uh) 2.1s
│ │ │ │ │ ● filesync 2.1s
dusk frigate
halcyon eagle
#

That must be because of the refactor on module loading then.

dusk frigate
#

i imagine this is due to module loading changes - the cli isn't doing find-up to work it out

halcyon eagle
#

I'll look into it.

dusk frigate
#

❤️ yeah, also happy to look into it, but figured it probably has a wonderful overlap with the fs navigation work

halcyon eagle
#

Yeah, does dagger query have the same issue?

dusk frigate
#

yup 😦

#

and dagger call

halcyon eagle
#

Dagger call is different. You need a module there.

#

In query, listen, and shell it's optional.

dusk frigate
#

true! but it does hang trying to upload my entire home dir, while it used to error out quickly.

#

less weird, but still a bit unexpected for new dagger users i guess

halcyon eagle
#

Yeah

#

Ping @old stirrup

#

We could keep a very simple implementation of find up in the client, just looking for a dagger.json in a parent directory. If not found, no reason to request ModuleSource.

dusk frigate
#

or maybe we could do find-up without needing to load the entire directory? feels like that might be somewhat possible, since find up works by stat-ing files

halcyon eagle
#

Does it really try to sync all files? Maybe the time it takes is because there's just too many directories?

dusk frigate
#

i think it's trying to copy them all:

upload /home/jedevc from b32t5hc5ezsulbudioavkyjil (client id: jjyd4o4c3ggeooqbsvq4i8cw7, session id: o08sruew63eklmkooxebeo3uh) 2.1s
#

at least from this message

halcyon eagle
#

Ugh... how can we check on a controlled directory, which files are being uploaded?

old stirrup
old stirrup
# dusk frigate and `dagger call`

how are you seeing the behavior with dagger call? When I run dagger call in an empty dir with no dagger.json in parents or anything I actually get a panic lol which is its own problem obviously

# 
Caught panic:

runtime error: invalid memory address or nil pointer dereference

Restoring terminal...

goroutine 15 [running]:
runtime/debug.Stack()
        /usr/lib/go/src/runtime/debug/stack.go:26 +0x64
runtime/debug.PrintStack()
        /usr/lib/go/src/runtime/debug/stack.go:18 +0x1c
github.com/charmbracelet/bubbletea.(*Program).recoverFromPanic(0x4000477a40)
        /go/pkg/mod/github.com/charmbracelet/bubbletea@v1.2.4/tea.go:705 +0x84
panic({0xf95ca0?, 0x24bc440?})
        /usr/lib/go/src/runtime/panic.go:785 +0x124
main.(*FuncCommand).loadCommand(0x24e0a80, 0x4000431508, {0x2532800, 0x0, 0x0})
        /app/cmd/dagger/functions.go:319 +0x29c
main.(*FuncCommand).execute(0x24e0a80, 0x4000431508, {0x2532800, 0x0, 0x0})
        /app/cmd/dagger/functions.go:294 +0x114
main.(*FuncCommand).Command.func2.1({0x13f4398, 0x40002fd4d0}, 0x0?)
        /app/cmd/dagger/functions.go:174 +0x188
main.(*FuncCommand).Command.func2.withEngine.2({0x13f43d0?, 0x4000405310?})
        /app/cmd/dagger/engine.go:63 +0x3a0
github.com/dagger/dagger/dagql/idtui.(*frontendPretty).spawn(...)
        /app/dagql/idtui/frontend_pretty.go:723
github.com/charmbracelet/bubbletea.(*Program).handleCommands.func1.1()
        /go/pkg/mod/github.com/charmbracelet/bubbletea@v1.2.4/tea.go:324 +0x64
created by github.com/charmbracelet/bubbletea.(*Program).handleCommands.func1 in goro
        /go/pkg/mod/github.com/charmbracelet/bubbletea@v1.2.4/tea.go:318 +0x10c

problem is this line, guessing mod or MainObject is nil https://github.com/sipsma/dagger/blob/250b95a11a0a711badea8ea6391c5d045219e879/cmd/dagger/call.go#L69-L69

#

I'll look to see what happens when I remove that condition that falls back to uploading the whole source dir

halcyon eagle
old stirrup
halcyon eagle
#

I expect call and functions to fail during module load if there's no module.

old stirrup
halcyon eagle
#

hmmm... initializeModule should fail if config doesn't exist. It's tryInitializeModule that shouldn't fail. That was the diff.

#

So basically initializeModule became tryInitializeModule but should be the other way around.

old stirrup
old stirrup
#

I definitely misunderstood what you were suggesting, sorry, it was getting hard to keep all the details in my head at once towards the end there

halcyon eagle
#

You can just return an error when !configExists in initializeModule to fix call and functions. I'll either push the fix for shell in your PR (quicker merge) or integrate in mine. The shell has two behaviors on initial load. If a module isn't found, it should load the core api only. It's like "try dagger call, fallback to dagger core".

#

The thing with the shell PR is that i didn't realize initializeModule wasn't failing. So I'll need to revisit that.

#

Actually the shell PR should be ok (in my PR, not in main). Because on init I use the maybeInitializeDefaultModule which already shouldn't fail and still exists. During run I make specific ModuleSource(ref).ConfigExists() calls before calling initializeModule, so those shouldn't fail.

#

If that wasn't clear: need to fix initializeModule to return error if !configExists. That will fix call, functions, and the shell in main. My shell PR doesn't change initializeModule, so this quick fix won't be undone later. It's actually needed.

old stirrup
#

I'll send a separate one for the default load of dir when no dagger.json found

#

I suspect that one might be a bit more tricky since there's more weird corner cases there

halcyon eagle
#

Yeah!

#

Curious, how do you plan on tackling that?

old stirrup
# halcyon eagle Curious, how do you plan on tackling that?

To start, going to remove the condition and push to a draft PR and see what tests fail 😄 I know there was some weird case involving a dagger.json getting deleted or something, but can't remember which test

From there, I guess there's a few approaches:

  1. Can defer module loading until later in that case. Fortunately the new code make it easy to load context whenever you want. It was nice to centralize it in moduleSource but implementation wise it can happen anywhere
  2. Maybe worst case we could use an extra arg to moduleSource to control this behavior for different use cases, but hopefully can avoid that
halcyon eagle
#

Yeah, or even maybe compile a list of includes based on parent dir + dagger.json for a first sync. That's the sort of thing I tried with the .gitignore support.

old stirrup
#

https://github.com/dagger/dagger/pull/9659 No tests failed? notsureif Highly possible that I added that to fix some test at some point and later fixed it in a different way. I later had to add some extra logic that reloads context whenever anything is changed that can impact it (withSourceSubpath, withIncludes, etc.)... I wonder if it's covered by those now?

GitHub
fix load of full dir when no dagger.json by sipsma · Pull Request ...

When there's no dagger.json found we are loading the full source root directory, which defaults to . if not specified. If the dir has a lot of files this makes hitting error cases slow and,...

dusk frigate
#

dagger v0.16.1, just in my homedir, fresh env, just running dagger call

#

i don't see the panic weirdly

dusk frigate
dusk frigate
#

hmmm, not strictly the same bug here, but also potential fallout from the same module loading:

echo "{version}" | dagger query
✔ connect 0.2s

✘ moduleSource(refString: "."): ModuleSource! 0.0s
! failed to unmarshal module config: unmarshal module config: unmarshal module config: unmarshal sdk as struct: json: cannot unmarshal number into Go value of type modules.alias

✔ version: String! 0.0s

{
    "version": "v0.16.1"
}
# 
❯ cat dagger.json
{
  "name": "test",
  "engineVersion": "v100.0.0",
  "sdk": 123
}
#

i have an intentionally invalid dagger.json, and it seems like we see it's invalid, but then just fallback to no-module load - i think ideally we'd only skip to the fallback of no-module load if the dagger.json doesn't exist?

#

(again dagger v0.16.1)

halcyon eagle
dusk frigate
#

intentional?

halcyon eagle
#

No, it should fallback only when dagger.json doesn't exist.

dusk frigate
#

okay cool 🎉

old stirrup
#

Is that a thing that needs a separate fix or is it covered by your shell PRs @halcyon eagle

halcyon eagle
#

Needs a fix, not covered.

#

My PR doesn't touch ModuleSource besides adding the original subpath.

#

There's another issue that throws a big wrench in the shell. This wasn't introduced by your refactor, but for the shell now these become different modules:

✔ .core | module-source github.com/dagger/dagger/sdk/php@v0.16.1 | as-string 8.0s
github.com/dagger/dagger/sdk/php@sdk/php/v0.16.1

✔ .core | module-source github.com/dagger/dagger/sdk/php/src@v0.16.1 | as-string 2.1s
github.com/dagger/dagger/sdk/php@v0.16.1
#

Notice the tags, sdk/php/v0.16.1 vs v0.16.1. I could use the digest instead of the as-string value as cache keys, but that adds the need for more calls. Right now the full ref is more helpful to diagnose (e.g., .debug). Would have to change a bunch of things to change that.

old stirrup
# halcyon eagle No, it should fallback only when `dagger.json` doesn't exist.

I guess the problem is that we just do the fallback behavior for dagger query and friends when err != nil, but we more want to only have the fallback behavior for some errors.

Matching on error string contents is gonna be a shitshow (though we can do that if nothing else)... I know we have the special ExecError that SDKs are aware of, but if I wanted to add more special error types is that something I can just do on the schema level and have all the SDKs pick up through codegen, or do special error types require per-SDK work to handle right now?

halcyon eagle
dusk frigate
old stirrup
old stirrup
#

eh nevermind, I realized that the place I put the "find-up cwd for named dep" logic was a little weird and that if I moved it a little down then I could actually just use the AllowNotExists option and skip typed errors: https://github.com/dagger/dagger/pull/9681

#

previously that find-up for named dep didn't run if allowNotExists: true, which was incorrect, so that's also fixed now

#

(I think, we'll see if there's some corner case I missed)

old stirrup