Skip to content
This repository has been archived by the owner on Jul 16, 2020. It is now read-only.

Context input type #26

Closed
kofalt opened this issue Mar 5, 2018 · 12 comments
Closed

Context input type #26

kofalt opened this issue Mar 5, 2018 · 12 comments

Comments

@kofalt
Copy link
Member

kofalt commented Mar 5, 2018

Frequently, there is need for configuration that is not really specific to the gear, and more specific to the organizational / logistical environment. Examples: matlab licensing key, project-specific override value.

Context values:

  1. Have no type
  2. Are not guaranteed to be present
  3. Are provided in the gear's config.json if found

Provisioning and lookup of context values are out of scope for the gear spec. An implementation could decide to provide context values any way it wants, or allow a user to modify it per-job.

For our implementation, the lookup process would walk up the hierarchy, checking container metadata for context.[VALUE], providing the first match if any and null if not found.

An example manifest excerpt:

"inputs": {

	// Normal file
	"dicom": {
		"base": "file",
		"type": { "enum": [ "dicom" ] },
		"description": "Any dicom file."
	},

	// Contextual key-value
	"matlab_license_code": {
		"base": "context"
	}
}

Results in config.json:

{
	"config": {
	},
	"inputs" : {
		"dicom" : {
			"base" : "file",
			"hierarchy" : {
				"type" : "acquisition",
				"id" : "5988d38b3b49ee001bde0853"
			},
			//...
		},

		// Context was found
		"matlab_license_code": {
			"base" : "context",
			"found": true,
			"value" : "aex"
		}
	}
}
@travisr
Copy link

travisr commented Mar 5, 2018

How could we use the same concept for config, but provide a default value if not found?

@kofalt
Copy link
Member Author

kofalt commented Mar 5, 2018

I suppose we could let the gear set a default value if the context is not found.

@travisr
Copy link

travisr commented Mar 5, 2018

Let's make sure we cover that. The idea is to enable a project, group, or site, to set their own defaults for use with a given gear. Ideally, we would still be able to edit them in the normal way when manually running a gear.

@nagem
Copy link
Member

nagem commented Mar 5, 2018

We should make sure any workflows we set up here aren't counter intuitive when gear config is added to rules: scitran/core#961

@nagem
Copy link
Member

nagem commented Mar 5, 2018

This would make context a "reserved" key on info then, correct? Or would context be a top level key?

@nagem
Copy link
Member

nagem commented Mar 5, 2018

Do we expect anything under context to be sensitive information or can anyone with permissions to the project view this information? Who is allowed to edit it? I'm wondering if it being stored directly on the container is the best option (would need to be filtered out on GETs) or if a using a separate collection with a foreign key to the container is a better solution.

@travisr
Copy link

travisr commented Mar 5, 2018

would be useful to have this mechanism be rich enough to save all default values for a given gear (possibly by having a "save as default" button on the config page of an analysis or on the gear run dialog. This would provide a project wide default for batch or manually run gears. It could also simplify entering the config attached to gear rules. The context variable would have to be nested to handle a set of values for each gear.

@kofalt
Copy link
Member Author

kofalt commented Mar 5, 2018

We should make sure any workflows we set up here aren't counter intuitive when gear config is added to rules: scitran/core#961

I would expect context to be provided the same way by a gear launched manually or automatically, so I don't see a conflict on that. It would have to be implemented in the rule launch logic.

This would make context a "reserved" key on info then, correct?

Correct, for some value of "reserved". Semantically meaningful, yes.

Do we expect anything under context to be sensitive information or can anyone with permissions to the project view this information? Who is allowed to edit it?

No, yes, and everyone with read access respectively.

would be useful to have this mechanism be rich enough to save all default values for a given gear (possibly by having a "save as default" button on the config page of an analysis or on the gear run dialog. This would provide a project wide default for batch or manually run gears. It could also simplify entering the config attached to gear rules. The context variable would have to be nested to handle a set of values for each gear.

I don't think this proposal is well set out to handle that, that sounds more like gear config templates (which I agree we should chase). That probably won't show up in the gear spec though.

@travisr
Copy link

travisr commented Mar 6, 2018

While I love the idea of defaults for gear config, I will back off for now. Let's just keep the scope of this to a basic set of environment variables under the context (or whatever we decide to call it)

@ryansanford
Copy link
Member

I do like any time we can eliminate the need to use the FW SDK within the gear. The utility with this proposal is impressive.

Is there danger in that utility creating a blurred line between config and input context? For example, as a gear author, why should I not be tempted to replicate all my gear config parameters as context objects as well to achieve the utility of project-level-gear-config?

I feel like we corrected a similar pattern where we incentivized gear authors to implement config default logic, due to the FW implementation not initially providing those defaults for gear rule jobs.

@kofalt
Copy link
Member Author

kofalt commented Mar 14, 2018

Is there danger in that utility creating a blurred line between config and input context?

I don't think so, but part of the intention is that it's pretty freeform. It's up to the implementation to either incentivize or deincentivize that as desired.

@kofalt
Copy link
Member Author

kofalt commented Mar 14, 2018

There's enough buyin from inside & outside the thread that I'm comfortable going forward. Anyone curious is welcome to comment on the upcoming PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

6 participants