Reduce Coupling to Definition Struct? about scs-build-client HOT 4 CLOSED

@tri-adam are you thinking that we treat build definition more as a bag-of-bytes?

@mikegray yeah, exactly.

So then instead of including nCPU/mem/architecture/singularity-version requirements possibly in this definition file, we need to define that as another input, right? Or are you thinking about this in some other way?

Agree with @ikaneshiro here, I think it best to keep these outside of the definition. In addition to nCPU/mem, I could see needing to specify the version of Singularity to use in the Build Service. In a way, this sort of thing is implicit when someone builds locally, but when building remotely, we need a way to provide this context to the Build Service. If we wanted to generalize this, perhaps we could make this a map[string]string? That would allow us to extend it arbitrarily in the future without having to update the signature.

from scs-build-client.

@tri-adam are you thinking that we treat build definition more as a bag-of-bytes? We take the input file (or []byte slice) and get it through to remote-build server-jim-manager-agent pipeline to have singularity execute it on the other end and we don't really ever need to know the structure of the file in between?

So then instead of including nCPU/mem/architecture/singularity-version requirements possibly in this definition file, we need to define that as another input, right? Or are you thinking about this in some other way?

@ikaneshiro would be interested in your thoughts here as well.

from scs-build-client.

@tri-adam @mikegray I agree with treating definitions as a bag-of-bytes. I think this will make it much easier for the builder to support building with multiple versions of singularity since it won't have to try to use different parsers for different structs internally. Are there any components of the builder that need information from the definition struct? From my quick look at the code it looks like it is mainly passed around until it is inserted in the build environment.

I think it's best to not include things like nCPU/mem/... as a part of the definition as it is only indented, imho, to be a list of actions for singularity to perform to create a container. These seem like things that are not used by singularity but jim to set up the build environment and should be handled separately.

from scs-build-client.

Closed in #17, but still more work to be done on backend to support this.

from scs-build-client.