8000 chore: convert withFile to dagop by alexcb · Pull Request #10569 · dagger/dagger · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore: convert withFile to dagop #10569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Conversation

alexcb
Copy link
Collaborator
@alexcb alexcb commented Jun 11, 2025

No description provided.

@alexcb alexcb force-pushed the dagop-withFile branch 8 times, most recently from b8e6133 to 94be28c Compare June 18, 2025 00:19
jedevc added 14 commits June 25, 2025 18:38
This is no longer allowed, since we'd require a dagop for this - we can
instead just re-call `withExec` through the server to get it working.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
We need to make sure that we pass this into the top-most context, since
it can be used during `Load`.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
This is useful, so now we can use dag.Select over it.

Signed-off-by: Justin Chadwell <me@jedevc.com>
We already have this implemented through the Server function. Don't
really know why it needed to be duplicated (seems like a past-me
brainfart).

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Various improvements, including:
- Allow configuring the entire buildkit cache map, instead of just the
  cache key
- Track the cause context.Context, useful for tracing (this is ideally
  where spans should be attached to)
- Fallback to getting client metadata using the weird hacky session
  manager thing that buildkit has. This is annoying, but if we don't do
  this, it's very easy to end up in strange edge merging bugs (and I
  don't want to remove that as part of this work).

Signed-off-by: Justin Chadwell <me@jedevc.com>
This helps to pass JSON-serializable objects through the schema easily.
It's not a directly exposed type, just a helper.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Reimplement it *entirely* inside of dagop!

There are a few different things components that this interacts with
that makes it a bit tricky to integrate.
- `asService` had a dependency on `ExecOp` - this needed to be entirely
  removed, which is a real pain for the legacy case, so we do
  an ID inspection.
- `ExecError` also depended on using `ExecOp`, so this needed reworking,
  it's now a `RichError` type which can be used for *any* sort of live
  ref debugging.
- Module calls, these for now still depend on using `withExec` in a
  super hacky way, but in the future, this should hopefully be removed
  as we cache module calls natively in dagql caching.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Alex Couture-Beil <alex@mofo.ca>
alexcb added 2 commits June 25, 2025 14:14
Regression test for case where mixing services and dagops would break
with an "open /.dagger_meta_mount: no such file or directory" error.

Signed-off-by: Alex Couture-Beil <alex@mofo.ca>
Explicitly return ErrNoCommand when meta is not set, rather than having
"empty reference" returned. This is to preserve existing functionality.

Signed-off-by: Alex Couture-Beil <alex@mofo.ca>
@alexcb alexcb 794B force-pushed the dagop-withFile branch from 1124724 to cf87763 Compare June 25, 2025 21:14
TUI no longer has a checkmark in it, use a dot instead.

Signed-off-by: Alex Couture-Beil <alex@mofo.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0