Update to latest Spin version with Factors #189
Conversation
| let mut futures_list = Vec::with_capacity(trigger_types.len()); | ||
| for trigger_type in trigger_types.iter() { | ||
| let app = spin_app::App::new("TODO", app.clone()); |
There was a problem hiding this comment.
How is app creation avoided in the Spin CLI? By directly invoking the triggers via commandline? How is app ID used? Should we just set the ID to be the locked app name for now?
There was a problem hiding this comment.
This seems to be something we might be able to change in upstream as that app id is rarely actually used. Currently, in the Spin code base it's only being used in the Redis trigger for telemetry purposes. It's used by some other runtimes simply as a unique identifier.
I've opened an issue for this: fermyon/spin#2852
In the meantime, do we have any sort of identifier we can use to globally identified the running application?
There was a problem hiding this comment.
By globally do you mean unique to the cluster too? Container name may be a good option
There was a problem hiding this comment.
The uniqueness isn't necessary (as far as I can tell) for everything to work properly - it is just needed for proper diagnostics as this string might be exposed to the user in telemetry. Therefore, I think we want something that makes sense to users if they see it.
I'm not sure what the best thing to use is - if you say container name works well that's fine by me. How do I access that information?
There was a problem hiding this comment.
We should be able to use the HOSTNAME env var to get the container name. This is not a unique container id; rather just the container name (ie busybox). Furthermore, if the container is using hostNetwork, the hostname becomes the node hostname, but in production, that is rarely done. For now HOSTNAME sounds like the best bet
There was a problem hiding this comment.
Did a quick check locally, printing the value of HOSTNAME from the shim and looks like it is the fully unique container name: "container name: simple-spinapp-59c4d9668-gz8dr". If you just run locally with ctr run it will use the node/machine hostname which is fine as that is not the normal execution path.
There was a problem hiding this comment.
@rylev what are your thoughts on using HOSTNAME?
Mossaka
force-pushed
the
refactoring
branch
from
7086cb3
to
79d54ba
Compare
|
Need a rebase |
Signed-off-by: Ryan Levick <[email protected]>
Signed-off-by: Ryan Levick <[email protected]>
Signed-off-by: Ryan Levick <[email protected]>
Signed-off-by: Ryan Levick <[email protected]>
Signed-off-by: Ryan Levick <[email protected]>
rylev
force-pushed
the
factors
branch
2 times, most recently
from
475f5cf
to
b9d5025
Compare
Signed-off-by: Ryan Levick <[email protected]>
|
@rylev can we mark this as "Ready for Review"? |
rylev
requested review from
jsturtevant,
radu-matei and
devigned
as code owners
|
@kate-goldenring I've marked this PR as ready to review, but we shouldn't merge until we've figured out the answer to this thread. |
Signed-off-by: Ryan Levick <[email protected]>
Signed-off-by: Ryan Levick <[email protected]>
This is ready for review, but it isn't quite ready to land as other PRs need to land first: