Fix odo overwrites devfile with flattened/resolved devfile and refactor component creation #5199
Conversation
openshift-ci
bot
added
do-not-merge/work-in-progress
valaparthvi
force-pushed
the
crefactor
branch
2 times, most recently
from
7a3b4cc to
8b71ea6
Compare
valaparthvi
force-pushed
the
crefactor
branch
from
8b71ea6 to
0337390
Compare
valaparthvi
force-pushed
the
crefactor
branch
from
18e8de8 to
28e9ec1
Compare
|
/retest |
valaparthvi
requested review from
feloy and
dharmit
and removed request for
rnapoles-rh
valaparthvi
force-pushed
the
crefactor
branch
from
28e9ec1 to
ade774c
Compare
|
✔️ Deploy Preview for odo-docusaurus-preview ready! 🔨 Explore the source changes: ade774c 🔍 Inspect the deploy log: https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6182c4d6ab7ad60008ad6a81 😎 Browse the preview: https://deploy-preview-5199--odo-docusaurus-preview.netlify.app |
|
/retest |
|
/test psi-kubernetes-integration-e2e |
valaparthvi
force-pushed
the
crefactor
branch
from
516d422 to
ade774c
Compare
|
/test psi-kubernetes-integration-e2e |
valaparthvi
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
valaparthvi
changed the title
|
/test psi-kubernetes-integration-e2e |
|
/test psi-kubernetes-integration-e2e |
|
/test psi-unit-test-windows |
| if err != nil { | ||
| co.createMethod.Rollback(co.componentContext) | ||
| return err | ||
| } | ||
|
|
||
| // From this point forward, rollback should be triggered if an error is encountered; rollback should delete all the files that were created by odo | ||
| defer func() { |
| } | ||
|
|
||
| func (icm InteractiveCreateMethod) Rollback(componentContext string) { | ||
| os.RemoveAll(componentContext) |
There was a problem hiding this comment.
Are you sure we want to remove all the files of the devfile context? The user could have source files in this place
There was a problem hiding this comment.
Philippe, what was the library you mentioned which could check diff in the directory. I am thinking to use it to decide what files should be deleted.
There was a problem hiding this comment.
I talked about this lib recently, but I'm not sure it is relevant: https://github.com/monochromegane/go-gitignore
But don't you think we just need to delete the devfile.yaml file here? The interface is about fetching this devfile.yaml only, I think it should only revert the creation of this file.
There was a problem hiding this comment.
On a second thought, I am not sure if deleting the starter project files is a good idea, we don't do it now. We can just delete the devfile and env files for now.
|
|
||
| // validateAndFetchRegistry validates if the provided registryName exists and returns the devfile listed in the registy; | ||
| // if the registryName is "", then it returns devfiles of all the available registries | ||
| func validateAndFetchRegistry(registryName string) (catalog.DevfileComponentTypeList, error) { |
There was a problem hiding this comment.
Yes, I think you could detect early that there is no registry configured and stop at this point
There was a problem hiding this comment.
Looks like this check has already been implemented.
valaparthvi
force-pushed
the
crefactor
branch
from
afaf7a3 to
738ae7d
Compare
|
SonarCloud Quality Gate failed.
|
|
/test psi-kubernetes-integration-e2e |
|
/lgtm Thanks for this refactoring, this will help for the maintenance of the create command |
openshift-ci
bot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: feloy 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 |
openshift-ci
bot
added
the
approved
What type of PR is this?
/kind cleanup
What does this PR do / why we need it:
Refactors the component/create.go
Which issue(s) this PR fixes:
Fixes #5112
PR acceptance criteria:
Unit test
Integration test
Documentation
I have read the test guidelines
How to test changes / Special notes to the reviewer: