Codecov Report

Merging #9308 (62d9b00) into master (0adf5ef) will increase coverage by 6.05%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##             master    #9308      +/-   ##
============================================
+ Coverage     63.64%   69.70%   +6.05%     
- Complexity     4984     5060      +76     
============================================
  Files          1809     1882      +73     
  Lines         96993   100177    +3184     
  Branches      14832    15232     +400     
============================================
+ Hits          61735    69831    +8096     
+ Misses        30707    25397    -5310     
- Partials       4551     4949     +398     

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

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Currently controller and server is using pinot..instance.id as the key, I'd suggest making broker and minion following the same pattern to be consistent

@Jackie-Jiang : We might need to deprecate pinot.broker.id as well in that case. Is that okay?

Currently controller and server is using pinot..instance.id as the key, I'd suggest making broker and minion following the same pattern to be consistent

@Jackie-Jiang : We might need to deprecate pinot.broker.id as well in that case. Is that okay?

@ankitsultana Per the current logic, I think we can directly modify the CONFIG_OF_BROKER_ID to pinot.broker.instance.id without worrying about backward compatibility because the field is always set within the BaseBrokerStarter and then read again

The field is read in BrokerAdminApiApplication and BaseBrokerRequestHandler.

But you are right, I took a look and both of these classes get the value from the one set by BaseBrokerStarter. I'll update the PR.

@Jackie-Jiang : Updated. One of the integration tests is failing with a "ServerSegmentMissing" error. Not sure if it is related.