-
Notifications
You must be signed in to change notification settings - Fork 71
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
Minimal project descriptor #32
Conversation
Signed-off-by: Joe Kutner <jpkutner@gmail.com> wip wip wip wip
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
0a48aaa
to
b500dda
Compare
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Updated based on feedback from 12/5/19 WG meeting |
Is the idea here that platforms would convert into the project descriptor from whatever they support for metadata files? Would unimplemented attributes raise an error? So a specific platform might extend the descriptor internally, for instance for extending os package support. |
We haven't defined how a platform will support this file. We hope that eventually it will make it's way into the spec. If it does, we generally believe that
No. The only error condition we define is when
Could this fit within the arbitrary |
# Summary | ||
[summary]: #summary | ||
|
||
This is a proposal for a new file, `project.toml`, containing configuration for apps, services, functions, and buildpacks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While .toml
is a great readable format, I have concerns that picking .toml
might add more friction to users coming from the container world, which seems to have converged towards using .yaml
. In my opinion, this is a data point that should be factored in when picking a format.
As a middle ground, maybe this proposal could say that .yaml
and .json
format are also accepted, but .toml
is preferred (in the end, they all represent structured data, and .yaml
is even a superset of .json
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should stick to toml
. My reasoning is that all other parts of the buildpack spec use yaml, and as a platform maintainer, it would be quite frustrating to have to support other formats both in the platform itself and in support requests coming in from users.
The name project.toml
also has fewer (no?) conflicts with other projects (I see that references to project.yaml
in OpenStack and ArgoCD).
The json
format doesn't allow for comments, which is why it wasn't picked.
version = "<string>" | ||
authors = ["<string>"] | ||
documentation-url = "<url>" | ||
source-url = "<url>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include
and exclude
are used by the build process. How are the other fields here intended to be used? (Are they turned into image annotations?) Should there be builder
and run-image
fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're not specifying any behavior associated with the other fields.
I think any option on pack
(including those you mentioned) is a good candidate for this descriptor. We batted around many different schemas, but are now trying to land on a very minimal subset of what was originally proposed.
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
@iainsproat i'm not sure if RFC0013 was ever implemented, but we should probably align this with it. However, I think the |
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
|
||
[[project.licenses]] | ||
type = "<string>" | ||
uri = "<uri>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nebhale for the non SPDX use case, do people usually point to a URI or just include a file to the LICENSE on disk locally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of the URI is typically handled orthogonally from whether there's an SPDX value or not. The customers that have asked us for this don't so much want a link to the boiler-plate of the license (the SPDX takes care of that in most systems), but rather proof that the project uses that license. So you'll see:
[[metadata.dependencies.licenses]]
type = "GPL-2.0 WITH Classpath-exception-2.0"
uri = "https://openjdk.java.net/legal/gplv2+ce.html"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I like this. It seems flexible for future expansion and maps more directly to lifecycle. I left some comments with things I'm not going to block on, but am interested in opinions on.
Signed-off-by: Terence Lee <hone02@gmail.com>
e06ee7d
to
077a1e4
Compare
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put some stuff in and would like to see some loosening of the requirements, but not enough to hold approval.
|
||
[[project.licenses]] | ||
type = "<string>" | ||
uri = "<uri>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of the URI is typically handled orthogonally from whether there's an SPDX value or not. The customers that have asked us for this don't so much want a link to the boiler-plate of the license (the SPDX takes care of that in most systems), but rather proof that the project uses that license. So you'll see:
[[metadata.dependencies.licenses]]
type = "GPL-2.0 WITH Classpath-exception-2.0"
uri = "https://openjdk.java.net/legal/gplv2+ce.html"
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
@nebhale i've addressed your feedback, and loosened the requirements on license type and uri. |
Used to set environment variables at build time, for example: | ||
|
||
```toml | ||
[[build.env]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would launch.env
be viable to be added as well? Thinking about environment variables that you want when your application launches that are outside of buildpacks (or override those provided by the buildpacks). Can be added later if we still need to discuss it further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think launch.env
is a bit more contentious, and we should discuss it in a separate update RFC.
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
I see now that this was merged in be3761d |
Readable
See also #25