8000 cli: don't serve transitive dependencies by vito · Pull Request #10581 · dagger/dagger · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

cli: don't serve transitive dependencies #10581

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

cli: don't serve transitive dependencies #10581

wants to merge 1 commit into from

Conversation

vito
Copy link
Contributor
@vito vito commented Jun 12, 2025

when loading a module by ref, only serve 8000 that module, otherwise it's easy to hit name conflicts (e.g. evaluator/workspace, evals/workspace)

@vito
cli: don't serve transitive dependencies
bb4906b 
when loading a module by ref, only serve that module, otherwise it's
easy to hit name conflicts (e.g. evaluator/workspace, evals/workspace)

Signed-off-by: Alex Suraci <alex@dagger.io>
@vito vito mentioned this pull request Jun 12, 2025
Merged
5 tasks
@helderco helderco requested a review from cwlbraa June 16, 2025 17:55
@helderco
Copy link
Contributor

Reference to original comment:

@cwlbraa @helderco Looking to double check this - without this fix, I ran into errors while trying to run this in shell, since there are two Workspace types:

With this fix applied it works fine:

This one seems redundant - we still pass IncludeDependencies: true for the primary module (that's how I'm able to call evaluator above), just not here.

I'll defer to @cwlbraa since he's the one that needed this for LLM prompts in:

Note that serving dependencies was also added to dagger query and dagger listen for consistency and removing this makes it inconsistent again, but we could add it as a CLI flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cwlbraa cwlbraa Awaiting requested review from cwlbraa

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vito @helderco
0