Remove Function CRD from porchconfig and Repo #119
Conversation
efiacor
requested review from
radoslawc,
kispaljr,
tliron,
liamfallon,
vjayaramrh,
kushnaidu,
Catalin-Stratulat-Ericsson and
JamesMcDermott
and removed request for
s3wong
| @@ -61,9 +53,6 @@ type RepositorySpec struct { | |||
| Deployment bool `json:"deployment,omitempty"` | |||
| // Type of the repository (i.e. git, OCI) | |||
| Type RepositoryType `json:"type,omitempty"` | |||
| // Content stored in the repository (i.e. Function, Package - the literal values correspond to the API resource names). | |||
| // TODO: support repository with mixed content? | |||
| Content RepositoryContent `json:"content,omitempty"` | |||
There was a problem hiding this comment.
Isn't this a backward incompatible change? What happens in cases when a repository is pushed in with a RepositoryContent: Package ?
There was a problem hiding this comment.
Good point. I can revert. Or is there a cleaner alternative?
What is the k8s procedure for deprecation of an api attribute?
There was a problem hiding this comment.
Won't the "Content" field just be ignored?
There was a problem hiding this comment.
Based on the OpenAPI spec in the CRD, it will be just ignored. Would be good to check manually at least.
There was a problem hiding this comment.
I suggested a potential change below that keeps the Content field the following way:
- if it is not specified explicitly its value is considered to be "Package", as before
- if it is specified with any value other than "Package", the creation of the
Repositoryobject will be rejected with a human readable error message stating that the field is deprecated.
Please note that make generate must be run after this change, since it also effects the CRD yamls.
Please note that I haven't tested the change below, I typed it directly into the GH comment box, so it most probably has typos.
|
/approve |
This is more part 1 of 2. Initial removal is from the porchconfig and Function Repo concept. |
|
Oh, ok. /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: efiacor, JamesMcDermott, nagygergo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
/test presubmit-nephio-go-test |
2 similar comments
|
/test presubmit-nephio-go-test |
|
/test presubmit-nephio-go-test |
|
test presubmit-nephio-go-test |
| @@ -61,9 +53,6 @@ type RepositorySpec struct { | |||
| Deployment bool `json:"deployment,omitempty"` | |||
| // Type of the repository (i.e. git, OCI) | |||
| Type RepositoryType `json:"type,omitempty"` | |||
| // Content stored in the repository (i.e. Function, Package - the literal values correspond to the API resource names). | |||
| // TODO: support repository with mixed content? | |||
| Content RepositoryContent `json:"content,omitempty"` | |||
There was a problem hiding this comment.
I suggested a potential change below that keeps the Content field the following way:
- if it is not specified explicitly its value is considered to be "Package", as before
- if it is specified with any value other than "Package", the creation of the
Repositoryobject will be rejected with a human readable error message stating that the field is deprecated.
Please note that make generate must be run after this change, since it also effects the CRD yamls.
Please note that I haven't tested the change below, I typed it directly into the GH comment box, so it most probably has typos.
efiacor
force-pushed
the
remove_function
branch
from
d007972 to
12f7597
Compare
|
/lgtm |
Function CRD is removed as it is unused.
Clean up RepoSpec function references.
Test clean up.
Further removal of the porch api Function attrs to follow.
#465