Skip to content

refactor: use common channel registry - #77

Merged
aaron-steinfeld merged 1 commit into
mainfrom
use-common-channel-registry
Sep 3, 2021
Merged

aaron-steinfeld merged 1 commit into
mainfrom
use-common-channel-registry

Conversation

@aaron-steinfeld

Copy link
Copy Markdown
Contributor

Description

The grpc channel registry was lifted from graphql and made common, and has since been improved to properly wait for shutdown to complete, which is a change we want to propagate back. Using a delegate model to leave the existing shutdown integration and save from changing all consumers.

Testing

Left existing UTs in place

@aaron-steinfeld
aaron-steinfeld requested a review from a team as a code owner September 3, 2021 14:47
@github-actions

This comment has been minimized.

@codecov

codecov Bot commented Sep 3, 2021

Copy link
Copy Markdown

Codecov Report

Merging #77 (a05b4db) into main (d941520) will decrease coverage by 0.20%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #77      +/-   ##
============================================
- Coverage     59.38%   59.17%   -0.21%     
+ Complexity      236      230       -6     
============================================
  Files            80       80              
  Lines          1199     1188      -11     
  Branches         34       34              
============================================
- Hits            712      703       -9     
  Misses          460      460              
+ Partials         27       25       -2     
Flag Coverage Δ
unit 59.17% <100.00%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...graphql/utils/grpc/DefaultGrpcChannelRegistry.java 100.00% <100.00%> (+12.50%) ⬆️

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 d941520...a05b4db. Read the comment docs.

@aaron-steinfeld
aaron-steinfeld merged commit 461fe95 into main Sep 3, 2021
@aaron-steinfeld
aaron-steinfeld deleted the use-common-channel-registry branch September 3, 2021 15:18
@github-actions

github-actions Bot commented Sep 3, 2021

Copy link
Copy Markdown

Unit Test Results

30 files  ±0  30 suites  ±0   31s ⏱️ -1s
91 tests ±0  91 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 461fe95. ± Comparison against base commit d941520.

private final Map<String, ManagedChannel> channelMap = new ConcurrentHashMap<>();
private volatile boolean isShutdown = false;

private final org.hypertrace.core.grpcutils.client.GrpcChannelRegistry delegate =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: style

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missed this in the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what specifically about the style (this was formatted by spotless)? the FQN?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes. never mind. i see it now.

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.

2 participants