Register and delete custom containers - proto - #892
Conversation
| string organization_id = 1; | ||
|
|
||
| // name format will be [user inputted name]:[first 8 characters of org id] | ||
| string name = 2; |
There was a problem hiding this comment.
Remind me why we need :[first 8 characters of org id]?
There was a problem hiding this comment.
It's to prevent possible collisions between custom containers and viam containers
There was a problem hiding this comment.
What kind of collision? How would such a collision come about
There was a problem hiding this comment.
The keys are name:version, so for example a collision would happen if a user had name=tf and version=2.16 since that matches one of the viam containers
|
|
||
| // Optional. Defaults to the image tag. Registered key is "name:version". | ||
| string version = 4; | ||
| string description = 5; // will serve as display name |
There was a problem hiding this comment.
I know we've already touched on this in at least one meeting, but it still seems odd to be to use description as a name. Would we run into a scenario where a user added a fairly long description, not realizing it was really a name, or will we always be masking the name of the field from them (so that they'll think they're providing the display name)? Or how will that be handled
There was a problem hiding this comment.
I see what you mean, John didn't want us to change it though since description is an existing field and changing it could cause issues. I am thinking we can add help text in the CLI to clarify things
…otics/api into register-delete-custom-containers
…t-custom-containers
Co-authored-by: Katie Peters <katiepeters@users.noreply.github.com>
…otics/api into APP-17611-list-custom-containers
…otics/api into register-delete-custom-containers
|
|
||
| message DeleteCustomTrainingContainerRequest { | ||
| string organization_id = 1; | ||
| string key = 2; |
| // visibility: VISIBILITY_PUBLIC | ||
| // created_on: 2026-01-15T01:30:15.01Z | ||
| // organization_id: "xyz" | ||
| // visibility: VISIBILITY_PUBLIC |
There was a problem hiding this comment.
Duplication?
| message RegisterCustomTrainingContainerRequest { | ||
| string organization_id = 1; | ||
|
|
||
| // On registration resolve URI to container's image:tag to set as key. |
There was a problem hiding this comment.
I don't think this comment is necessary?
| // Docker Hub reference, tag required: docker.io/<namespace>/<image>:<tag>. | ||
| // Official images normalize to docker.io/library/<image>:<tag>. |
There was a problem hiding this comment.
I'm trying to make sense of these comments. Would it be true to say this:
// Docker Hub image reference. Must include a tag; references without one
// are rejected at validation (no implicit :latest).
//
// The registry prefix is optional on write and normalized on read:
// <image>:<tag> -> docker.io/library/<image>:<tag>
// <namespace>/<image>:<tag> -> docker.io/<namespace>/<image>:<tag>
| // On registration resolve URI to container's image:tag to set as key. | ||
| // Docker Hub reference, tag required: docker.io/<namespace>/<image>:<tag>. | ||
| // Official images normalize to docker.io/library/<image>:<tag>. | ||
| string image_uri = 3; |
There was a problem hiding this comment.
Is there a reason image_uri is 3 and description is 5?
| } | ||
|
|
||
| message RegisterCustomTrainingContainerResponse { | ||
| Container container = 1; |
There was a problem hiding this comment.
Returning the id might be the better convention?
| } | ||
|
|
||
| message DeleteCustomTrainingContainerRequest { | ||
| string organization_id = 1; |
There was a problem hiding this comment.
If deleting by id, we do not need to include organization_id
Published version of viamrobotics/api#892, which adds RegisterCustomTrainingContainer and DeleteCustomTrainingContainer to MLTrainingService. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
https://viam.atlassian.net/browse/RSDK-14444