Skip to content
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

container funcs to return dev friendly warnings #1319

Merged
merged 3 commits into from
Jul 26, 2022

Conversation

gaurav-gogia
Copy link
Contributor

No description provided.

@gaurav-gogia gaurav-gogia requested a review from a team as a code owner July 8, 2022 09:57
@@ -100,15 +100,15 @@ func fetchContainersFromAwsResource(resource jsonObj, hclBody *hclsyntax.Body, r
}
fileData, err := ioutil.ReadFile(fileLocation)
if err != nil {
zap.S().Errorf("error fetching containers from aws resource: %v", err)
zap.S().Warnf("error fetching containers from aws resource: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is just a warning, we probably shouldn't return the control. Also the messages clearly says error

@codecov-commenter
Copy link

Codecov Report

Merging #1319 (a1bb8b6) into master (313ccf3) will not change coverage.
The diff coverage is 25.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1319   +/-   ##
=======================================
  Coverage   77.39%   77.39%           
=======================================
  Files         277      277           
  Lines        7813     7813           
=======================================
  Hits         6047     6047           
  Misses       1406     1406           
  Partials      360      360           
Impacted Files Coverage Δ
...ders/terraform/commons/extract-container-images.go 78.63% <25.00%> (ø)

@@ -126,7 +126,7 @@ func getContainersFromhclBody(hclBody *hclsyntax.Body) (results []output.Contain
for _, arg := range funcExp.Args {
re, diags := arg.Value(nil)
if diags.HasErrors() {
zap.S().Errorf("error fetching containers from aws resource: %v", getErrorMessagesFromDiagnostics(diags))
zap.S().Warnf("error fetching containers from aws resource: %v", getErrorMessagesFromDiagnostics(diags))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaurav-gogia let's change this msg as well, instead of error fetching container make it failed to fetch the container since we are making it a warning keeping the error message will create confusion. WDYT

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@nasir-rabbani nasir-rabbani merged commit a9f115a into tenable:master Jul 26, 2022
@gaurav-gogia gaurav-gogia deleted the update/logging branch September 5, 2022 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants