-
Notifications
You must be signed in to change notification settings - Fork 601
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
Don't swallow errors in Lineage package #8401
Don't swallow errors in Lineage package #8401
Conversation
Signed-off-by: Ali Ok <[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.
@aliok can we make this behavior configurable? It will enable tools to either configure "skip unauthorized resources" (so that we could build partial graphs scoped to the user access) or "fail fast on unauthorized resources" (the latter seems a good default)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8401 +/- ##
==========================================
- Coverage 64.22% 64.12% -0.11%
==========================================
Files 388 388
Lines 23302 23339 +37
==========================================
Hits 14965 14965
- Misses 7542 7579 +37
Partials 795 795 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Ali Ok <[email protected]>
for i := range sourcesList.Items { | ||
unstructuredSource := sourcesList.Items[i] | ||
duckSource, err := duckSourceFromUnstructured(&unstructuredSource) | ||
if err == nil { | ||
duckSources = append(duckSources, duckSource) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to convert unstructured source to duck source: %w", err) | ||
} | ||
|
||
duckSources = append(duckSources, duckSource) | ||
} | ||
} |
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.
I made a behavior change here.
Before:
- Ignore the issue that the unstructrued source cannot be converted to a duck source
After:
- Do not ignore it, even with
Lenient=true
I've introduced a field called Behavior change: #8401 (comment) |
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: aliok, pierDipi 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 |
Fixes #
Proposed Changes
Lenient:bool
(wanted to have the default tofalse
, thus, a positive field name versusFailFast
). If this field is set to false, there will be no behavior changes, except one place (noted below).Pre-review Checklist
Release Note
Docs