SDA-6670 | feat: add formating function to retrieve output logs #182
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #182 +/- ##
==========================================
+ Coverage 10.03% 10.08% +0.05%
==========================================
Files 17 17
Lines 1146 1140 -6
==========================================
Hits 115 115
+ Misses 1021 1015 -6
Partials 10 10
|
pkg/output/output.go
Outdated
| } | ||
|
|
||
| // Parse returns the data being stored on output | ||
| // - failures as []error | ||
| // - exceptions as []error | ||
| // - errors as []error | ||
| func (o *Output) Parse() ([]error, []error, []error) { | ||
| return o.failures, o.exceptions, o.errors | ||
| func (o *Output) Parse() ([]error, []error, []error, []string) { |
There was a problem hiding this comment.
I would rather that we create a new function to retrieve debug logs from this output struct rather than continue to extend this Parse() function.
However, maybe I don't understand the problem - what is missing that this PR hopes to achieve? The description isn't super clear
There was a problem hiding this comment.
Oh, sorry. We actually don't really need the Parse at all, we could just not use it. I was mostly aligning it so we can have debug logs access
The goal is to be able to access debug logs from clients, CS for instance. As currently it has no access
Also adding a formatted string output instead of printing to stdout so it is easier to consume the logs and the control of the format can be handled within network verifier itself.
Just easing support so we can move forward uploading logs to s3 buckets
There was a problem hiding this comment.
Gotcha, yeah if Format() is all you need that would be great. If we find that we need Parse() I think we had better consider just making separate functions for each of:
debugLogs []string
// failures represents the failed validation tests
failures []error
// exceptions is to show edge cases where a verifier test couldn't be ran as expected
exceptions []error
// errors is collection of unhandled errors
errors []error
or just making all of them public members in the struct
There was a problem hiding this comment.
Cool, removed Parse changes for now. We can handle it later if necessary
|
@gdbranco: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gdbranco, mjlshen 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
Related issue: https://issues.redhat.com/browse/SDA-6670
What
Adds a formatted string output from logs instead of directly printing to stdout, clients can consume the raw string
Why
Clients currently have no access to the debug logs