Skip to content

Conversation

@owent
Copy link
Member

@owent owent commented Aug 15, 2024

Fixes #3026

Changes

Please provide a brief description of the changes here.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@owent owent requested a review from a team August 15, 2024 15:11
@codecov
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.62%. Comparing base (497eaf4) to head (35ce020).
Report is 119 commits behind head on main.

Files Patch % Lines
...metrics/export/periodic_exporting_metric_reader.cc 80.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3030      +/-   ##
==========================================
+ Coverage   87.12%   87.62%   +0.51%     
==========================================
  Files         200      190      -10     
  Lines        6109     5872     -237     
==========================================
- Hits         5322     5145     -177     
+ Misses        787      727      -60     
Files Coverage Δ
...metrics/export/periodic_exporting_metric_reader.cc 79.81% <80.00%> (+0.46%) ⬆️

... and 122 files with indirect coverage changes

@marcalff marcalff changed the title Fix singal missing. [SDK] PeriodicExportingMetricReader: future is never set, blocks until timeout Aug 16, 2024
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

Copy link
Member

@esigo esigo left a comment

Choose a reason for hiding this comment

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

LGTM
thanks for the PR :)

@esigo esigo added the ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve) label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SDK] PeriodicExportingMetricReader: future is never set, blocks until timeout

4 participants