Enable generic sub-resources #76
Conversation
Signed-off-by: Karol Zadora-Przylecki <karolz@microsoft.com>
| @@ -101,7 +102,7 @@ func (c completedConfig) New() (*TiltServer, error) { | |||
| } | |||
|
|
|||
| // Add new APIs through inserting into APIs | |||
| apiGroups, err := buildAPIGroupInfos(c.ExtraConfig.Scheme, c.ExtraConfig.Codecs, c.ExtraConfig.APIs, c.GenericConfig.RESTOptionsGetter) | |||
| apiGroups, err := buildAPIGroupInfos(c.ExtraConfig.Scheme, c.ExtraConfig.Codecs, c.ExtraConfig.APIs, c.GenericConfig.RESTOptionsGetter, c.ExtraConfig.ParameterCodec) | |||
There was a problem hiding this comment.
Custom ParameterCodec allows to define kind- and sub-resource-specific parameters for REST requests.
pkg/server/builder/resource/types.go
Outdated
| Name() string | ||
| // TODO: fill the details for this interface. | ||
| GetStorageProvider(parentObj Object, rootPath string, fs filesystem.FS, parentWatchSet *filesystem.WatchSet, parentSP apiserver.StorageProvider) apiserver.StorageProvider |
There was a problem hiding this comment.
To allow for custom storage implementation via GetStorageProvider method, I moved and made public a few types from the filepath package to filesystem package.
I initially started with an implementation that was using the FS and WatchSet types, but ended up relying on parent storage pretty much exclusively, so I am happy to revert that part if necessary. I decided to leave it in this PR because I thought other sub-resource types might need access to these interfaces more than mine.
There was a problem hiding this comment.
hmmmm....how is your implementation using fs and parentWatchSet?
The Kubernetes apiserver APIs for resources are usually more high-level than this, and don't usually require specific details about how the underlying objects are stored... but maybe i'm missing something here...
There was a problem hiding this comment.
I agree. When I looked at the PR just after I submitted it, I thought it probably makes more sense to solely rely on the parent storage. Let me rework this PR and verify that my subresources still work, but I think they will. This will also eliminate the need to change the filepath/filesystem types.
There was a problem hiding this comment.
Done. The change is much simpler/smaller now and still allows me to do everything I need for DCP.
pkg/server/builder/resource/types.go
Outdated
| Name() string | ||
| // TODO: fill the details for this interface. | ||
| GetStorageProvider(parentObj Object, rootPath string, fs filesystem.FS, parentWatchSet *filesystem.WatchSet, parentSP apiserver.StorageProvider) apiserver.StorageProvider |
There was a problem hiding this comment.
hmmmm....how is your implementation using fs and parentWatchSet?
The Kubernetes apiserver APIs for resources are usually more high-level than this, and don't usually require specific details about how the underlying objects are stored... but maybe i'm missing something here...
Signed-off-by: Karol Zadora-Przylecki <karolz@microsoft.com>
|
@nicks I was able to simplify the proposed change quite a bit by not passing FS/WatchSet to subresource storage factory method. Please take another look--much appreciated! |
|
Thank you! 🤗 |
This change enables defining a resource type that has arbitrary ("generic") sub-resource. I used it to prototype a "log" sub-resource for a couple of kinds used by .NET Aspire orchestrator a.k.a. DCP (which is built on top of Tilt API server). The way they work via a custom storage implementation that exposes a stream of data.
@nicks and I talked briefly about this kind of enhancement to Tilt API server last year, but not since, so I understand this might come as a bit of a surprise. I am open to any suggestions for how to improve this PR, or enable the scenario differently.