Watch for devfile dependencies #6020
Conversation
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
feloy
force-pushed
the
feature-5867/get-devfile-deps
branch
from
128ada9 to
c21cae3
Compare
feloy
force-pushed
the
feature-5867/get-devfile-deps
branch
from
c21cae3 to
9334e83
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if u.Scheme == "" { |
There was a problem hiding this comment.
Just curious - is the file URI scheme something possible here to reference local files? Or should we always expect files relative to the devfile.yaml? The Devfile spec is pretty vague about this:
Location in a file fetched from a uri.
There was a problem hiding this comment.
I'm pretty sure it is related to the directory containing the devfile. For example, the doc for parent URI is:
URI Reference of a parent devfile YAML file. It can be a full URL or a relative URI with the current devfile as the base URI.
| for _, f := range devfileFiles { | ||
| err = devfileWatcher.Add(f) | ||
| if err != nil { | ||
| klog.V(4).Infof("error adding watcher for path %s: %v", f, err) |
There was a problem hiding this comment.
Shouldn't we return the error right away instead? Or maybe make the error more visible to the user by logging it as a warning?
There was a problem hiding this comment.
I first started with returning the error, but lots of integration tests fail, because they are using Devfiles containing URIs but withtout the corresponding file.
I would prefer the error to be handled by the devfile validation. I consider that if the devfile validation does not complain, we should not here.
openshift-ci
bot
added
the
lgtm
|
/approve |
|
@valaparthvi: Overrode contexts on behalf of valaparthvi: windows-integration-test/Windows-test In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: valaparthvi 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
This reverts commit 04dec85.
* GetReferencedLocalFiles * Watch for all devfile dependencies * Integration test
What type of PR is this:
/kind bug
What does this PR do / why we need it:
Which issue(s) this PR fixes:
Fixes #6017
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer: