-
Notifications
You must be signed in to change notification settings - Fork 36
Filtering devfile obj via attributes #46
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
Conversation
Signed-off-by: Maysun J Faisal <[email protected]>
pkg/devfile/generator/utils.go
Outdated
@@ -197,7 +198,7 @@ func getServiceSpec(devfileObj parser.DevfileObj, selectorLabels map[string]stri | |||
|
|||
var containerPorts []corev1.ContainerPort | |||
portExposureMap := getPortExposure(devfileObj) | |||
containers, err := GetContainers(devfileObj) | |||
containers, err := GetContainers(devfileObj, common.DevfileOptions{}) |
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.
should we add DevfileOptions
to getService as well? to filter out endpoints from specific components / with specific attributes for service creation
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.
updated to incl options to service but endpoint will be discussed and added later if necessary since its a level 2 attribute
Signed-off-by: Maysun J Faisal <[email protected]>
Signed-off-by: Maysun J Faisal <[email protected]>
pkg/devfile/generator/utils_test.go
Outdated
@@ -926,7 +1040,7 @@ func TestGetPortExposure(t *testing.T) { | |||
Components: tt.containerComponents, | |||
}, | |||
} | |||
mapCreated := getPortExposure(devObj) | |||
mapCreated, _ := getPortExposure(devObj, tt.filterOptions) |
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 returned error should be checked
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.
true it should be checked. The reason i didnt because err will only be returned when using attributes and i found a bug with attributes in the api repo and doesnt return err properly but i will update this
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.
updated
@@ -191,13 +278,19 @@ func TestDevfile200_UpdateCommands(t *testing.T) { | |||
|
|||
d.UpdateCommand(tt.newCommand) | |||
|
|||
commandsMap := d.GetCommands() | |||
commands, _ := d.GetCommands(common.DevfileOptions{}) |
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.
same as above, the error should not be ignored. should check unexpected error
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.
same as above
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.
updated
@@ -213,7 +249,7 @@ func TestGetDevfileContainerComponents(t *testing.T) { | |||
}, | |||
} | |||
|
|||
devfileComponents := d.GetDevfileContainerComponents() | |||
devfileComponents, _ := d.GetDevfileContainerComponents(tt.filterOptions) |
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.
same as above, the error should not be ignored. should check unexpected error
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.
same as above
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.
updated
Signed-off-by: Maysun J Faisal <[email protected]>
Signed-off-by: Maysun J Faisal <[email protected]>
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.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maysunfaisal, yangcao77 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 |
Signed-off-by: Maysun J Faisal [email protected]
What does this PR do?
Provides an option to filter devfile object via attributes
For example, this helps Console filter components that its interested in rather than parsing all the components belonging to a devfile.yaml
What issues does this PR fix or reference?
Fixes devfile/api#232
Is your PR tested? Consider putting some instruction how to test your changes
TBD