-
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
RFC: Project Descriptor(project.toml) Converter #182
RFC: Project Descriptor(project.toml) Converter #182
Conversation
Maintainers, As you review this RFC please queue up issues to be created using the following commands:
Issues(none) |
b27bcb2
to
ac18c92
Compare
for DCO I force pushed. and fixed conflict errors because of force push. |
Looks like you might have accidently signed too many commits back. The commits tab should only include your changes. |
9a44d5e
to
abfe08d
Compare
Thanks for making me realize,
I hope problem is fixed. Edit: I found better way;
|
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
abfe08d
to
f5bddf3
Compare
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 Project TOML Converter is a CLI tool that will ship alongside the lifecycle.
Why are we deciding to make this a separate binary? As opposed to another code path that gets run? Is this a preemptive move for a ./prepare
phase binary?
--
I guess... I'm not sure that this is a problem that has to be solved. While I agree with the underlying premise:
We think it's too hard for platform operators to support multiple versions of project.toml.
the behavior that I'm expecting is that the platform complains/rejects about deprecated project schema versions. My thoughts:
- There's plenty of other tooling that exhibit this behavior.
- Having the internal component of the platform silently upgrade my input not only feels like black magic, but a bit over-engineered.
- A future schema version might indeed be a breaking change, which would require user input before making a schema upgrade
- And finally, keeping in mind that the application developer is writing these things, I'm of the opinion that they should be prompted to upgrade by the platform. Otherwise, this RFC encourages this non-best-practice of using
schema v0.1
forever.
But this is a good discussion that we should be having. Thanks for raising this RFC, @haliliceylan
This is a valid question, the only input I have from a platform's perspective is that we'd want to get access to only this conversion operation without any other side effects. As an example, take the field How to access the functionality. There are a couple options for this:
Not sure I find that as a good justification.
Based on my desired mentioned above (see "How to access the functionality") I'm not sure I'm seeing a lot of black magic.
Could you provide an example? Anything I can think or we've experienced in the past has had reasonable defaults or ways to mitigate.
The problem that I think we're trying to solve is the need for platforms to all be "in sync". This sort of functionality is to provide more interoperability between platforms. For example, I want my app to work (using project.toml) on pack, tekton, GCP, etc. It would be a bad experience if GCP told me to use schema A but pack said schema B. |
These are great points in response to my original comment. Thank you for raising them. In general, once more, perhaps I don't see the "the need for platforms to all be in sync". To me, a parallel to the project.toml is the AWS Cloudformation Template. The template itself has a Format Version, with only one valid entry. We can assume that AWS will not convert my template for me. This is the behavior I've come to expect - even if other platforms accepted AWS Cloudformation templates. I just wanted to point my predilection based on conditioning as a cloud tool user. It looks like this project, and maintainers, has a better perspective on how platforms should behave. And that's completely fine. |
|
||
- This feature has similarities with the "prepare" phase that has been discussed previously (see #555). But it is much smaller in scope. Potentially, the binary described in this RFC could eventually take on more responsibilities and look more like "prepare". But that is out of scope for this RFC. | ||
|
||
- Project Descriptor (aka project.toml) is a file which describes behavior related to buildpacks. It is non-trivial for platform operators to upgrade support for different versions of project.toml. So with this feature we are giving to developers the ability to use any version of project.toml knowing that the platform can always support it. At the same time we are giving to operators the convenience of only having to know about one schema at a time for project.toml. |
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.
What operators are we talking about here? I have the sense that this should say platform implementers. If that is so then I have a predicament with this statement. Only a subset of the project.toml
wouldn't be translated due to the fact that the spec allows for undefined keys/tables. It seems like platforms may very well still have to care about the initial input format to some degree.
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.
project = _ | ||
build = io.buildpacks | ||
build.buildpacks = io.buildpacks.group | ||
metadata = _.metadata |
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.
what happens to overlapped properties? ie. translating the following from v0.2
to v0.1
:
[_]
name = "My App"
# in this schema version this is consider arbitrary/user data
[project]
name = "Client 2 Project"
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.
maybe we can add a global prefix or postfix for keys which not included this list ? (just an idea)
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.
5c2075d should help clarify the mapping from 0.1 <-> 0.2 ...though I wasn't sure what to do about [build.buildpacks.script]
.
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.
IIRC, we were going to ignore (not map) previously non-existent properties. It doesn't make sense for platforms to expect properties that aren't associated with their specification.
text/0000-project-toml-converter.md
Outdated
url = "https://cdn.example.com/assets/foo.jar" | ||
checksum = "3b1b39893d8e34a6d0bd44095afcd5c4" | ||
|
||
buzz = ["a", "b", "c"] |
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.
If I recall correctly, arbitrary data would be retained. Could we get an example of a 1-to-1 real complex example to get a better understanding on how the translation would work?
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.
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.
5c2075d may help
metadata = metadata | ||
``` | ||
|
||
For conversion to/from v0.2: |
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 should call out explicitly that we only intend to "translate" data under _
and io.buildpacks
. Then there is the question of what to do with data under other domains - I believe that
- the original project.toml in 0.2 schema
- a separate file for "other" stuff
- a separate file for each domain - but this would be hard because we wouldn't know which
.
to split on
were all discussed. I don't think there is much substantive difference between the first two, and my preference is for the first one for its simplicity.
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Halil İbrahim ceylan <acolops@gmail.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano do you mind if I move this blocked dependent on the resolution of #191? |
Let's draft this as we can safely say it is superseded by #202 |
Readable
Hello everyone,
This is my first RFC as a GSOC Mentee at this summer. We prepared this RFC with @natalieparellano. We spent some time in my own fork to get ready this RFC. You can access previous comment and feedbacks from here.