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
Allow calling core functions from the CLI #6947
Comments
FYI @sipsma, this may be a duplicate, after a quick search I couldn't find a direct equivalent, but may not have found it. |
Would you use
|
Good question... any thoughts @sipsma @vito @aluzzardi @jedevc ? |
Good point - I like the idea of needing to explicitly select it, so yes, we'd probably need to reserve this ASAP. |
Reserving |
Would it break |
Actually it doesn't. Just need to choose a different name if installing so that you can't use Basically, only the exact |
Wdyt of |
I think the downside of both options, is that the module name doesn't map to the codegen namespace.
I know that you don't actually have to install core, but conceptually it is preinstalled. So the mismatch is potentially confusing. Also if we ever want to support multiple |
Let’s see, the main difference when loading core vs a module named “core” is this:
Just brain dumping here to make a point… The default for
Those would avoid a naming conflict, but are they intuitive? The thing is, I don’t think anyone is going to “guess” to use End brain dump As for using multiple Just trying to see if there’s value in trying to keep this as a possibility. But to be clear, I don’t think supporting calling core functions hurts this ability. |
Throwing another option into the mix: Might be a little odd but it'd at least be leaning on pre-existing GraphQL terminology that we already have to reckon with, as opposed to "core" which makes sense but comes with the cost of claiming another word. Maybe that could even work with multiple dagger -m github.com/vito/daggerverse/apko -m query call apko --help
# equivalent to:
dagger -m github.com/vito/daggerverse/apko call --help Though it's not obvious what utility that has, since it's not like modules can monkey-patch anymore. |
What about |
I thought about that but we need something to avoid naming conflicts like |
What's the naming conflict we need to avoid? |
Just disambiguity. If you have a Just trying to avoid having to reserve a name if there's something we can use to disambiguate. |
The output in #6947 (comment) has been reduced because of So all that remains is to finish bikeshedding on what value to use for The You can use |
I prefer cc @sipsma |
I see too problems though:
Footnotes
|
That's true, but not a dealbreaker IMO. We should assume there will be more "special" modules in the future (ie. a stdlib) so we will probably need a way to list those modules in the future anyway.
This problem isn't limited to the CLI though, is it? For example, what happens if I install a module named "container" today, and then try to call TLDR: yes name conflicts with core modules are a problem, but I think a problem we already have and should solve separately. |
I put in a footnote that you wouldn't be able to create a module with that name but you can:
Both cases allow you to use this today: ❯ dagger -m container functions
Name Description
container-echo Returns a container that echoes whatever string argument is provided
grep-dir Returns lines that match a pattern in the files of the provided Directory What's better?
I prefer checking core first before falling back to normal module loading. It's more performant, but potentially confusing. However I don't think anyone's going to actually be in this situation. It's possible but unlikely. |
|
If the module with the dependency has sources, it'll fail to |
In that case, I think it's fine to also fail in the same situation in the CLI. Consistency is probably better. For the subdirectory case, I still think core should win (because it's easy to disambiguate), but if it's easier to do it another way, I can live with that too. TLDR: I think both edge cases can be handled, even if I'm not 100% sure how. And I think the better UX of WDYT? |
I'll just look for a match under As for the UX, I'll just miss the list of core functions, but we do need a separate feature to list modules (DEV-3629), for example to list installed modules that you can run directly. There's also no way to display the module's description today so this could be it. In terms of consistency, doing Even though a user module adds a field to the root type, we've been talking about the core module as being the group of all other functions under "Query". Does it make sense that Some core modules don't return an object with functions, which violates the assumption that a module's constructor returns the module's main object type. In the case of There's also the I don't think these are deal breakers, I'm just doing a barometer check on the terminology. If everything is a module, it's natural that some things in the core API will feel like fitting a square peg in a round hole because it wasn't a design we had from the start. |
Personally, I kind of think core as a module, and Here's a few concrete reasons I think of it that way:
To me, it feels like there's a lot to unpack around how multiple core modules would work - and I think if we were to do it, it would ideally require a bit of restructuring of the APIs themselves to make this feel planned + neat (which is disruptive). I think being able to call core APIs from the CLI is more important to be able to do sooner-rather-than-later. Is there anything stopping us from having a In regards to where we attach the methods - I could imagine a future deprecation where we move all of our core module methods to |
Not technically.
Yeah, that was my thinking too but the current design was growing on me. Let me lash this out and make a summary of the options. RationaleIt’s the core API itself that doesn’t map all that well to the concept of modules. The current state of the core module was introduced in a Groundwork for exposing core API in CLI PR, but it doesn’t mean that design is finalized. Especially with discussions of a possible stdlib module. So if we do end up grouping it all under Another way to solve the mismatch is to decouple module loading with constructor selection. The - dagger --mod=parrot call parrot echo --msg=hello
+ dagger --mod=parrot call echo --msg=hello But now we have a mismatch with core functions. We can solve that by decoupling module loading with constructor selection, using perhaps a dagger --mod=parrot call --dag=parrot echo --msg=hello This way, if the default for dagger --mod=parrot call echo --msg=hello But would allow you to specify a core function instead (note that this way, "container" doesn’t need to be a module): dagger --mod=parrot call --dag=container from --address=alpine Granted, in this case it’s a waste to load the user module so you’ll likely want one or the other: dagger call --dag=version However, since dagger --mod="" call --dag=version But… at this point, if every time you need On one hand, the CLI could clear dagger --mod="" call version So
Which still feels consistent to me. It’s at this point that I tried finding something a little more ergonomic and ended up liking this one better: dagger -m- call version Now if we turn core functions into modules, but keep putting dagger -m version call More of a stretch to explain. Bikeshedding options
After re-considering those options, I’m leaning more towards Using |
Problem
There is no way to call core functions from the CLI
Solution
Allow calling core functions from the CLI.
See https://daggerverse.dev/mod/github.com/shykes/core for a userland implementation.
The text was updated successfully, but these errors were encountered: