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.

Sign up for GitHub

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

Jump to bottom

Make MonitoredResourceUtil#getValue not throw NPE on missing/malformed values #514

Closed
wants to merge 1 commit into from
Closed

Make MonitoredResourceUtil#getValue not throw NPE on missing/malformed values #514

wants to merge 1 commit into from

Conversation

artem-karpenko
Copy link

Fixes #512 ☕️

@artem-karpenko
fix: make MonitoredResourceUtil#getValue uniformly not throw exceptio…
2ae8be6 
…n when value is missing or is not of expected form
@artem-karpenko artem-karpenko requested review from a team as code owners April 28, 2021 20:30
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 28, 2021
@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/java-logging API. label Apr 28, 2021
@codecov
Copy link

codecov bot commented Apr 29, 2021 •
edited

Codecov Report

Merging #514 (2ae8be6) into master (c1862df) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #514      +/-   ##
============================================
- Coverage     79.32%   79.27%   -0.06%     
  Complexity      745      745              
============================================
  Files            44       44              
  Lines          4252     4255       +3     
  Branches        286      287       +1     
============================================
  Hits           3373     3373              
- Misses          692      695       +3     
  Partials        187      187
Impacted Files Coverage Δ Complexity Δ
...om/google/cloud/logging/MonitoredResourceUtil.java 37.67% <0.00%> (-0.80%) 5.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1862df...2ae8be6. Read the comment docs.

simonz130
simonz130 suggested changes Apr 29, 2021
View reviewed changes
google-cloud-logging/src/main/java/com/google/cloud/logging/MonitoredResourceUtil.java
@@ -188,7 +192,7 @@ private static String getValue(Label label, String resourceType) {
try {
value = new String(Files.readAllBytes(Paths.get(filePath)), StandardCharsets.UTF_8);
} catch (IOException e) {
throw new LoggingException(e, true);
value = null;
Copy link

Choose a reason for hiding this comment

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

This needs to have a test

@minherz
Copy link
Contributor

minherz commented Oct 12, 2021

Duplicates #708

@minherz minherz closed this Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonz130 simonz130 simonz130 requested changes

Assignees

@simonz130 simonz130

Labels
api: logging Issues related to the googleapis/java-logging API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MonitoredResourceUtil#getResource non-uniformly throws exceptions on missing/malformed values
3 participants
@artem-karpenko @minherz @simonz130