docs: component constructor ADR#110
Conversation
|
Even though the PR is not fully ready it has enough content to be reviewed |
e119c0d to
bbf9f65
Compare
9e98e9c to
9541888
Compare
462d17d to
9f1b889
Compare
Skarlso
left a comment
There was a problem hiding this comment.
A few questions really. Overall the design is okay. Though I will read this again.
7debada to
ef6374f
Compare
Skarlso
left a comment
There was a problem hiding this comment.
A few more questions for clarification. Otherwise, LGTM honestly.
frewilhelm
left a comment
There was a problem hiding this comment.
In general very nicely written. Thanks a lot!
Maybe what I am missing are examples why the extensibility is beneficial. Why should the model be extensible? What input types are likely to be implemented?
14cb6e5 to
a7654d2
Compare
see the ADR for a concrete description
a7654d2 to
aa5a5f5
Compare
frewilhelm
left a comment
There was a problem hiding this comment.
I was wondering if by reference-accesses would download the referenced resource as well to calculate the resource digest to put it in the CV. This is the current behaviour of ocm lib v1. However, this is not mentioned in this ADR.
There several arguments for downloading the resources (or getting the resource digest through an OCI manifest) and against it. For instance, secure delivery vs. separation of concerns vs. substantial changes in the cli usage for users...
Is the discussion or "feature" also in scope for this ADR or should we postpone it?
56b2fd9 to
9b7ec49
Compare
I agree, I ammended a section on the ADR to describe specifically how to handle digest behavior through a digest provider. The advantage of a dedicated digest provider is that we can more efficiently compute digests: while in the past OCMv1 had to download entire resources, I propose that the underlying implementation can decide how to amend the digest. For OCI for example, the implementation would just need to resolve the descriptor.
For me secure delivery means resources should be able to be provided with a digest for our most supported accesses. But for extension purposes we should probably go for a flow such as above. I ammended the PR as such with a separate section. Also note that this only applies to resources, because sources do not have digests.
definetly IN SCOPE for ADR, but implementation may need to be done in steps. PTAL at the changed ADR @frewilhelm and thanks for bringing it up! |
9b7ec49 to
b85e040
Compare
b85e040 to
05588db
Compare
|
Very nice! Thanks a lot. |
What this PR does / why we need it
see the ADR for a concrete description or the issue for a short summary
Which issue(s) this PR fixes
fix open-component-model/ocm-project#501