Conversation
Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>
1006722 to
b9abe16
Compare
rylev
left a comment
There was a problem hiding this comment.
Looks fine to me - I'm curious if there's anyway to expose some of this logic around component retention in Spin so that the spinkube doesn't need to copy the logic.
@rylev i agree that this is entirely duplicated code. I brought up this topic in the Spin PR but we punted not knowing the best place for it: spinframework/spin#2826 (comment). That being said, I'll look at moving it to a crate |
|
@Mossaka @devigned @jsturtevant @radu-matei would love your thoughts on this. |
|
I don't have a strong opinion about this particular change. It makes sense and is minimally invasive. The point about duplicating code is probably going to continue without a better option. I can't recall the context of the conversation, but at some point recently I believe @Mossaka and I were discussing if many of these changes could be avoided by exposing the I think extending the |
Mossaka
left a comment
There was a problem hiding this comment.
LGTM.
One concern that i have is that this seems like a lot of sharing code between here and spinframework/spin#2826 and this smell tells me that there is an opportunity to extract them to a library and share between these two projects.
|
@Mossaka, @rylev brought up the same point that i addressed here: #197 (comment) |
@devigned, I always thought changing/duplicating |
I felt the same way about duplication of the
I think these are some interesting cases which deserve some consideration. |
@devigned have you looked at the component dependencies work @fibonacci1729 has brought into Spin? It enables referencing upstream components in your @Mossaka for continuing this discussion in an issue, I feel like Spin may be the best place for that since we are talking about OCI artifact contents |
|
Merging it rn. |
|
oh oops, I forgot to mention, can you add this change to the CHANGELOG, starting with Unreleased section? 🙏 |
Is this something we will do for all PRs? I am not sure I understand making that a manual process rather than generating a CHANGELOG on release or having someone do it all at once. Is this a best practice you've seen in other projects? I can see how it is nice to be able to see what the changes are on main easily. |
Spin CLI added support for
spin up --component-id "foo" --component-id "bar"to run a subset of components of a Spin app in spinframework/spin#2826.This brings that functionality to the shim assuming that the components to support are supplied in
SPIN_COMPONENTS_TO_RETAINenvironment variable of a Pod as a comma separated list (SPIN_COMPONENTS_TO_RETAIN="foo,bar").In the Spin Operator, we can add a field to the SpinApp CR spec called
componentsthat lists which components of an app to retain in a pod.This is on top of @rylev's changes #189. The additive changes are in 1006722