Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Installing a dev container feature more than once in a devcontainer.json #44

Open
joshspicer opened this issue Jun 17, 2022 · 16 comments
Open
Labels
proposal Still under discussion, collecting feedback
Milestone

Comments

@joshspicer
Copy link
Member

joshspicer commented Jun 17, 2022

The current dev container features specification proposal is based on an array-like syntax for the 'features' attribute, and thus allows for the invocation of a single feature >1 with different options.

An execution goal stated in the spec is that

"It should be possible to execute a feature multiple times with different parameters."

We are currently discussing how best to tackle feature installation order (#43), with the discussion leaning toward proceeding without use of an array, meaning that there is a need to explore other mechanisms for invoking a feature to install >1 times.

Proposal

The propose extending the value to include an array of objects.

{
    "image": "azcr.io/joshspicer/my-docker-image",
    "features": {
        "devcontainers/features/dotnet": [
            {
              "version": 3.1,
              "runtimeOnly": true,
              "installUsingApt": true
            },
            {
              "version": 6.0,
              "runtimeOnly": false,
              "installUsingApt": false
            }
        ]
    }
}

This devcontainer.json will invoke the devcontainers/features/dotnet feature twice with two sets of options in order (first with the 3.1 set of arguments, and then 6.0 set of arguments).

@joshspicer joshspicer changed the title Installing a devcontainer feature more than once in a devcontainer.json Installing a dev container feature more than once in a devcontainer.json Jun 17, 2022
@joshspicer joshspicer added the proposal Still under discussion, collecting feedback label Jun 17, 2022
@Chuxel
Copy link
Member

Chuxel commented Jun 17, 2022

This seems like a reasonable proposal to me and meets some of the things @edgonmsft has highlighted - @chrmarti ?

@chrmarti
Copy link
Contributor

Another option is to extend feature options, such that, they can capture arrays and objects of values. That would allow features to capture feature-global options, the list of versions and version-specific options depending on the tools they install.

@jkeech
Copy link
Contributor

jkeech commented Jun 27, 2022

My preference is to keep it as simple as possible and not overcomplicate things. I like the proposed array syntax with the full options object inside if we are sticking with an object at the top level for the set of features.

Note that this requires using the same version of the feature for each execution (E.g. devcontainers/features/node@2 would be used for both executions). Do we need to support installing with one feature version with one set of options and another feature version with another set of options? Does the current spec allow specifying the same feature with different versions in the key with the current object syntax, such as the following?

{
    "features": {
        "devcontainers/features/node@1": {
            "version": "14"
        },
        "devcontainers/features/node@2": {
            "version": "16",
            "someNewOptionInV2": "..."
        }
    }
}

@joshspicer
Copy link
Member Author

joshspicer commented Jun 27, 2022

@chrmarti - are you suggesting features that look like this?

"features": {
   "devcontainers/features/[email protected]" : {
       "version": [16, 14, latest]
   }

or equivalently with the shorthand that implies "version"

"features": {
   "devcontainers/features/[email protected]" : [16, 14, latest]
}

How would a user modify >1 option for a given feature? I could imagine wanting a user to install a feature >1 time that uses any permutation of options.

@joshspicer
Copy link
Member Author

Does the current spec allow specifying the same feature with different versions in the key with the current object syntax, such as the following?

Yes, given the object syntax and these are two unique keys, I think we should (and would have to) support a declaration like this.

@edgonmsft
Copy link
Contributor

I also the array syntax so that the whole options object can be modified for each run.

The version question is an interesting one. The object syntax does allow us to use different versions since they would have different ids, although ordering will probably get interesting and the user would probably need to override it.

@joshspicer
Copy link
Member Author

For the dotnet feature, I can imagine users wanting to install dotnet twice with the following options.

These are the existing options today, and two plausible ways a user could configure several installations of the feature.

{
  "version": 3.1,
  "runtimeOnly": true,
  "installUsingApt": true
}
{
  "version": 6.0,
  "runtimeOnly": false,
  "installUsingApt": false
}

image

@Chuxel
Copy link
Member

Chuxel commented Jun 27, 2022

@joshspicer @edgonmsft @jkeech @chrmarti

Yeah, to that point - Personally I think we're talking about two separate proposals here.

  1. I think that we will need an array of strings option type. Right now if we added the ability to install a list of packages for a feature like homebrew, you'd need to do a space separated list which isn't great. So an array of strings is a reasonable type. It's also an example I wouldn't want to use this proposal for.
  2. This proposal is going beyond those basic needs and trying to meet the need of varying sets of configuration options for a given feature. You can't resolve that with just an array option type. Something along these lines will likely be needed as well. That is what Josh is showing in Installing a dev container feature more than once in a devcontainer.json #44 (comment).

With that in mind, I'd propose we not discuss this as an "or", but rather whether we think (2) this is needed now. We clearly can see the (1) is needed at a minimum to cover cases we know about. An alternative is also that we implement both at once.

@joshspicer
Copy link
Member Author

joshspicer commented Jun 27, 2022

@Chuxel I agree we may be getting side-tracked. I added a new issue to address (1) in your comment: #57

I want this proposal to focus on (2), particularly for the reason I highlighted in this comment. This is the first example I could come up with - I think this will be a more generic problem, and I think the syntax I propose is a good solution to that problem.

@joshspicer
Copy link
Member Author

This is the current interface for the python feature

image

@chrmarti
Copy link
Contributor

If we allow for more flexibility in the options, features can handle both: multiple versions and more advanced options. Always having an object for a feature also seems to keep the schema simpler. (Not sure if the thinking was to loop over the feature's install.sh, but in general it will be more flexible and potentially less error-prone to let the feature decide how to best handle multiple versions.) The object also allows for global options, e.g., install_jupyterlab might only make sense to have once.

{
	"features": {
		"devcontainers/features/[email protected]": {
			"versions": [
				{
					"version": "3.5",
					"optimize": true
				},
				{
					"version": "3.8",
					"install_python_tools": false
				}
			],
			"defaultVersion": "3.5",
			"install_jupyterlab": true
		}
	}
}

I think we should disallow having the same feature in multiple (feature) versions. That will make it easier for feature authors because they do not need to consider that case.

@jkeech
Copy link
Contributor

jkeech commented Jun 28, 2022

@chrmarti how would complex objects get passed in and parsed by the install script? I think it will end up being more complicated for both end users and feature authors to deal with complex option types and instead that we should support executing a feature multiple times by looping on the outside with different sets of simple options.

@chrmarti
Copy link
Contributor

This seems simple for the end-user. For the feature we can continue to pass in a flattened list of attributes (possibly omitting the nested ones) and also include a variable with the full options JSON. I expect most features to require only simple options.

@alexdima
Copy link
Member

alexdima commented Jul 18, 2022

We agree:

  • we want to proceed with the proposed array syntax
  • it is our goal that features are idempotent / re-entrant / don't explode if they are executed multiple times and we operate under this default.
  • a feature should be able to declare in its manifest if it is safe to be executed multiple times or not. The default is that it is safe to be executed multiple times, but a feature should have a way to indicate the contrary.
  • when running the "Add Development Container Configuration Files..." command, the features already installed in the base image should be indicated in the quick pick and potentially not suggested via ctrl+space.
  • the feature test command could ensure features don't explode if they are executed multiple times.

@Alegrowin
Copy link

Meanwhile, is there any workaround for something like this?

{
    "features": {
		"ghcr.io/devcontainers-contrib/features/asdf-package:1": {
			"plugin": "aws-vault"
		}
       		"ghcr.io/devcontainers-contrib/features/asdf-package:1": {
			"plugin": "consul"
		}
    }
}

@joshspicer
Copy link
Member Author

@Alegrowin There's no workaround within the spec today to install the same Feature twice. We're exploring changing this soon with the introduction of composite/dependencies, but this exact functionality in on our backlog (although I agree it's functionality that would be very nice to have).

Today, a lot of the Features in the wild wouldn't behave nicely if installed twice (either short-circuit early or overwrite existing installs). I personally don't want to introduce a solution the right guidance in place for authors to make the behavior predictable.

For this example, I think a Feature that proxies to another plugin manager should naturally accept >1 plugin. For example, I think it would be nice if upstream the feature worked like so:

{
    "features": {
		"ghcr.io/devcontainers-contrib/features/asdf-package:1": {
			"plugin": "aws-vault,consul"
		}
    }
}

With Feature dependencies though, without rigid rules around options we'd still need to support the case you're illustrating above (installing the Feature twice). And I also understand that someone may want to use a Feature as-is, even if it's a bit ugly.

Thanks for putting this back on our radar as useful to you. Taking this feedback back to our team.

nogic1008 added a commit to nogic1008/WritableOptions.Net that referenced this issue May 11, 2023
nogic1008 added a commit to nogic1008/WritableOptions.Net that referenced this issue May 17, 2023
@joshspicer joshspicer removed the active label Jun 19, 2023
nogic1008 added a commit to nogic1008/WritableOptions.Net that referenced this issue Aug 11, 2023
nogic1008 added a commit to nogic1008/WritableOptions.Net that referenced this issue Dec 29, 2023
@joshspicer joshspicer removed their assignment Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Still under discussion, collecting feedback
Projects
None yet
Development

No branches or pull requests

8 participants