Proposal for Features functionality and spec. #27
Conversation
| | name | string | Name of the feature/definition | | ||
| | description | string | Description of the feature/definition | | ||
| | documentationURL | string | Url that targets the documentation of the feature. | | ||
| | licenseURL | string | Url that points to the license of the feature. | |
There was a problem hiding this comment.
Can we add an OSI boolean?
I will be faster to filter then regex on license remote files.
There was a problem hiding this comment.
Main issue might be that we would have to get authors to maintain that flag. It might be easier to allow the value to be the name of an OSI approved license OR an URL to a custom license (maybe as an object literal that has an OSI flag for those who want/have to link to their copy of an OSI license).
A license name to show in the UI would also make sense.
There was a problem hiding this comment.
The scope was related to having something machine readable so that we can quckly filter also in the UI.
How can we quickly filter OSS only elements?
There was a problem hiding this comment.
@edgonmsft Do I need to open a new ticket to track this?
There was a problem hiding this comment.
Yep, lets create a new one. I think its a good idea the we should flesh out a little bit more. Thanks!
proposals/devcontainer-features.md
Outdated
| | metadata | any | Freeform data added by users for their own purposes. | | ||
| | keywords | array | List of keywords relevant to a user that would search for this definition/feature. | | ||
| | install.app | string | App to execute.ll (Optional, defaults to /bin/sh) | | ||
| | install.file | string | Parameters/script to execute. (Defaults to validate.sh) | |
There was a problem hiding this comment.
Would that be needed for all script files? Let's omit this for simplicity, the predefined names do not seem to be a limitation.
There was a problem hiding this comment.
Not sure I get what you mean.
There was a problem hiding this comment.
We can always use install.sh (assuming validate.sh is a typo), there is no need to make this configurable.
There was a problem hiding this comment.
Bein configurable is so that features can be created in other languages, if its not present we do fallback to install.sh
There was a problem hiding this comment.
I would avoid adding non-essential properties to the spec for now and this can be covered by having an install.sh forward the call.
proposals/devcontainer-features.md
Outdated
|
|
||
| From a practical point of view, features are folders that contain units of code with different entrypoints for different lifecycle events. | ||
|
|
||
| Features can be defined by a `feature.json` file in the root folder of the feature. The file is optional for backwards compatibility but it is required for any new features being authored. |
There was a problem hiding this comment.
Let's use a distinctive name to make it easy to associate a schema file (and possibly other editing support) to it. devcontainer-feature.json would work well. Similarly, let's use devcontainer-feature-collection.json or shorter devcontainer-features.json.
There was a problem hiding this comment.
I liked the shorthand of just feature.json but if we agree on that change, I can make it.
There was a problem hiding this comment.
The shorthand is nice, but I think tools support will be easier when we use a more distinct name.
There was a problem hiding this comment.
Made the changes. cc @joshspicer, @samruddhikhandale
|
|
||
| The application that implements features should: | ||
|
|
||
| - Features are executed in the order defined in devcontainer.json |
There was a problem hiding this comment.
We should keep this declarative for several reasons:
- Usability: When there are implicit dependencies the user has no way knowing what they are and we have no way of changing them over time without risking to break existing configurations.
- Parallelization: We want to preserve the option to parallelize installation in the future, using multi-stage builds or otherwise.
- Explicit dependencies: We want to preserve the option to add explicit dependencies, these would change the order of installation (potentially change it over time as dependencies change).
One example was the creation of a user: Since the user is specified in the devcontainer.json as a top-level option, it makes more sense to deal with that in general instead of as part of a feature (the common feature?). It might be easiest if we create the user before installing any features if that user does not exist yet.
There was a problem hiding this comment.
- I like the idea of having/using the cached features for things like users, although I'm not sure if defaulting to running it before other features is the best idea.
I feel like there are two ways of dealing with this. One is the array format which makes it clear to the user in what order things are going to run and the user has total control of that. Not sure I like the idea of things changing from under me if there is an update somewhere upstream. Declaring dependencies could actually be done indevcontainer.jsoninstead of as part of the feature itself. If we do that the tool can more easily parallelize, but it complicates the user experience in that now they have to declare the dependencies instead of just using an in order execution.
Thoughts? @Chuxel @jkeech @joshspicer
There was a problem hiding this comment.
To clarify: Dependencies would be declared by features themselves (not in the devcontainer.json), the user should not have to know about these. We don't have dependencies at the moment, but since we have already investigated adding them, I use them as one example to show that having "features" in the devcontainer.json determine an execution order will constrain us in the future. (Even more important is the usability argument above.)
On the user example: The user is configured with the top-level "remoteUser" independently of features. That suggests that we should address it also independently of features. Since features then depend on that user, we would create it before we run the feature install scripts. (Having a feature create the "remoteUser" doesn't seem to make sense since that user needs to exist also without any features for the configuration to be valid.)
Another argument for keeping "features" declarative is that it will make adding auto-detection of features in the future simpler. (If order matters, we need logic to arrange that for the detected features.)
There was a problem hiding this comment.
Ok, I get what you mean but not sure if we should add that to this proposal till we have a better idea of how we want it to work.
There was a problem hiding this comment.
The main idea of making this changes is to allow users to control the order of execution, especially since we are not supporting dependencies yet and the changes to the OS applied by features might clash between themselves in unpredictable ways that a user might need to fix in this way.
Thoughts @Chuxel, @bamurtaugh, @jkeech, @joshspicer ?
There was a problem hiding this comment.
I'm not sure if this would work, but is there a way a user could set a property if the order of their features matters? I think the concerns @chrmarti mentions make sense, but I can see the value in what @edgonmsft mentions about controlling the order of execution, so could we default to declarative but allow setting a property that ordering matters, in case users would like to override this declarative default?
There was a problem hiding this comment.
The order that works best will depend on how the features are implemented and could change over time. The user does not have any insight into this. When we add dependencies we want to be in control of the order.
We should ensure the order of installation is always the same (e.g., sort by id), so the outcome is reproducible. If we have any implicit dependencies today, I suggest we take a closer look and find a way to deal with them short term.
There was a problem hiding this comment.
I disagree, I think the user being able to execute things in the order they choose is important for the following reasons:
- Running things like
pip installandnpm install getc that need to run after their respective sdks are installed. This can also be custom user code that needs to explicitly run after certain features. - There won't be a feature for everything, users need to be able to add their own code and control where it executes in the order with the minimum of fuss. Likewise, it should be easy to know at a glance how things are working.
- Dependencies actually are on the underlying changes to the OS and not on features themselves. A feature can have its dependencies satisfied in different ways, like if the base image the user selects already has it installed in a different way. There can also be multiple features that satisfy an underlying dependency in a different way.
- In order for features to be more reusable it is easier to avoid making monolithic features and allow users to go in and out of the ecosystem as they need, this would also provide a path for users to grow from features to more custom builds without having to get out of using features and needing to replicate what they do.
- At some points in time there might be issues with existing features when new images are released and users need a way to fix by running code before, in the middle or after some features.
- I actually think its a bad idea if a change in a feature changes the order of installation completely without the user knowing about it.
We've already encountered the case where to build some complex images we need to add custom code at different points in time, code that only makes sense for that image and not as a reusable feature.
There are probably more reasons than this, thoughts? @chrmarti, @Chuxel, @joshspicer , @jkeech
There was a problem hiding this comment.
My biggest concern with obscuring the installation order of features (which are implemented as shell scripts with no standardization on how to author them), is that is will significantly hinder the ability of a community member to author their own set of features, and prevent the wider adoption and ecosystem of features.
We have not landed on a simple, complete mechanism for automated versioning/dependency management despite a fair bit of time and effort researching and discussing options. We've investigated building on top of existing package managers like npm. Our biggest detractor for such an idea is that we aren't just installing the feature, but the feature with the provided options. Our "blessed" set of features today already function quite differently given provided options - We would need strict authoring rules for the community, as well as a strictly defined set of metadata and listing of dependencies, to make the declarative features object feasible. We are talking about reimplementing a package manager like 'apt'.
I think that is too complicated given the problem 'features' is trying to solve (An easy-to-use mechanism to get tool X available in your dev environment, that can auto-update on rebuilds if you want it to)
To the three arguments for implicit ordering:
Usability: When there are implicit dependencies the user has no way knowing what they are and we have no way of changing them over time without risking to break existing configurations.
I think the opposite is true. We provide the user with total control over dependencies (we hope there are few with features, but maybe there are 1 or 2). I think hiding the dependencies will cause more breaks for users.
Parallelization: We want to preserve the option to parallelize installation in the future, using multi-stage builds or otherwise.
This argument makes sense, we lose a bit of control over installation order if we aren't able to decide that ourselves.
Explicit dependencies: We want to preserve the option to add explicit dependencies, these would change the order of installation (potentially change it over time as dependencies change).
This is what we are seeking with the array syntax, an option for users to add explicit dependencies. If a dependency changes significantly, it is on the author to create a new major version of the feature and for users to make the conscious decision on how they would like to proceed in their dev containers. I have already implemented a way to pin to a GitHub release or tgz URL, ranges are a next logical step.
There was a problem hiding this comment.
Additionally, we'll also need to take a dependency on the base image, which could be based on a yet another base image, both installing tools. While I think, in a perfect world, we could write tooling to figure everything out for a user, pragmatically I don't think we want to get into that business :D
| The application that implements features should: | ||
|
|
||
| - Features are executed in the order defined in devcontainer.json | ||
| - It should be possible to execute a feature multiple times with different parameters. |
There was a problem hiding this comment.
- Should a feature opt-in to supporting multiple runs? Otherwise this will make writing features harder.
- Is it up to the feature to decide which run will be on the PATH by default?
- How would a feature provide a version switcher to change which install is on the PATH?
There was a problem hiding this comment.
- I agree that features can opt-in to supporting multiple runs, perhaps as a propertie of
devcontianer-feature.jsonthat we could validate in some way. - While making changes to the implementation of the current features I added parameters to control it and with true. That reinforces the idea that features need to declare if they support multiple runs or not.
- I feel like that is more a feature of a tool that a user would use at runtime.
There was a problem hiding this comment.
It would be easier to let a feature add options for installing multiple versions (maybe with some additional support for doing that). That would allow the feature to control installation of all requested versions in a single run instead of having it run multiple times. With that approach IntelliSense in the devcontainer.json would likely include a hint if a feature supports multiple versions (e.g., an "additionalVersions" property).
There was a problem hiding this comment.
Yeah I don't want to block features from receiving multiple values and using them to loop, the idea is that we should also not disallow multiple runs.
|
I think this is ok as an initial version, thoughts? @Chuxel, @bamurtaugh, @joshspicer, @samruddhikhandale , @chrmarti |
|
Yeah I think as a proposal we could get it in and then have another PR with updates if that's ok w/@chrmarti and @bamurtaugh as well. One formatting note above 👆 |
|
Sounds good to me! This plus #33 can serve as great initial versions of the spec, which we can update in subsequent PRs as we make changes and get community feedback. |
|
Just pushed the comments. |
|
Merging as a proposal and we can do additional PR's for more changes before adding it to the main spec. |
After the beta functionality of features included today and with the feedback we have received so far, here is a proposal to define Features and how the work, get community feedback and create a formalized spec that can be implemented.