-
Notifications
You must be signed in to change notification settings - Fork 634
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
🐞 existing services break in 0.15 #9190
Comments
Hi @levlaz Thank you for reporting this issue. I just tried this with v0.14.0 and got the same error:
once I added could you please confirm if that makes sense or did I missed something when trying to reproduce the problem? |
I think I know what might be happening here. when calling @jedevc is that expected? We may have to add a legacy vs non-legacy version of |
Hm, yes. |
It actually might be possible to do something like this - instead of needing to handle a legacy function: var svc dagql.Instance[*core.Service]
err := s.srv.Select(ctx, ctr, &svc,
dagql.Selector{
Field: "asService",
View: s.srv.View,
},
) But that's just a guess, we need a test for this as well. |
I actually tried it, and it seems like it always pass the server view if we use |
Is this not what we want? |
The server is created from the module source, so I would expect this to work. We already use it like this: dagger/dagql/introspection/types.go Lines 21 to 24 in 3503abc
|
hmm I must be missing something (will take a short break and try again), but it seems to be returning v0.15.2 when I use I also tried adding a logline to engine when this api is called, and that prints v0.15.2 as well. |
If you call this in the context of a module - it should give the right result though. |
I confirmed by adding logging that the view is set to v0.14.0 (from dagger.json) when calling the function that returns a container. BUT when we call I am trying to identify how to fix this. |
@rajatjindal what's the command you're running to see this? Are you doing (if so, then yes, the Only modules are given the backwards compat guarantee - the CLI always consumes it's version of the API. There was a point when I considered versioning the CLI like this - but if you do this, you can very easily get problems where the CLI expects a specific API version for it's own internal queries. |
Okay okay, I synced with @rajatjindal on this, and we worked out next steps. So - the reason this breaks is this:
Aside from the fixes we'll make below - I think it's worth updating snippetbox to return a Note Why can't we just use the API versioned from the module in the CLI? Why can't we just have This is a deliberate design decision we made when doing module compat - without this, it's really weird. Suppose we make a breaking change to an API in That said, the current state is horribly difficult to migrate to, so here's what we're going to do:
I think this is the best we can do, keeping the API design that we've done - sadly, I don't think there's a feasible way to get the snippetbox module working with v0.15.0 as-is, with no changes to either the module or the The aim is to try and fix this ASAP, and get it into v0.15.2 sometime early this week. |
Thx for the writeup Jeremy, it's super clear.
Should we expand this a bit and briefly explain why? |
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
PROPOSAL
Said another way: For Terminology note:
Users being bit by need for The Dagger philosophy: In most cases, for the best containerized pipelines-as-code experience, users should not rely upon the OCI container image’s Note: In order to have compatibility with the Docker and Kubernetes's ecosystems, users have full control over setting the In practice, we should support: Note: If both Note: Calling [1] [2] Note today in 0.15.1, you need to use: |
@shykes Looks like we have consensus on this proposal ☝️ from most of team and I know we discussed it live where I believe you were 👍. Could you drop a comment here to record decision to re-introduce the prior behavior of:
|
@jpadams @marcosnils @levlaz I opened a separate issue specifically for the proposed API change (better for the record): #9305 |
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes dagger#9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
* engine: return error if service images can't be started. this addresses the issue introduced in v0.15.0 where services will hang to start if neither CMD or ENTRYPOINT is used. fixes #9190 Signed-off-by: Marcos Lilljedahl <[email protected]>
What is the issue?
I have a sample go application (https://github.com/levlaz/snippetbox) that installs mariadb (https://daggerverse.dev/mod/github.com/levlaz/daggerverse/mariadb@cea1668da940b45864116049bd20087855c8c787) from the daggerverse and mounts it as a service binding.
THe mariadb module is targeting 0.11.6 and the snippetbox module is targeting 0.13.16
This is no longer working in 0.15 with the following error:
I suspect its not able to get the service to work due to the new API change, but I was under the impression that this change should not break existing pipelines like this.
Dagger version
dagger v0.15.0 (registry.dagger.io/engine:v0.15.0) darwin/arm64
Steps to reproduce
dagger -m github.com/levlaz/snippetbox call server
Log output
Here's a trace: https://dagger.cloud/levs-test-org/traces/f06d326bb79ffd99fb7afdc647b58f21
The text was updated successfully, but these errors were encountered: