Skip to content

Restart extractor on config change#655

Open
Toshad wants to merge 7 commits intomasterfrom
config_restart
Open

Restart extractor on config change#655
Toshad wants to merge 7 commits intomasterfrom
config_restart

Conversation

@Toshad
Copy link
Copy Markdown

@Toshad Toshad commented Apr 9, 2026

Currently, we restart on config change only if restart policy is 'always' but we also want to restart for other cases to support integrations.

@Toshad Toshad requested a review from a team as a code owner April 9, 2026 06:13
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.98%. Comparing base (8724278) to head (19b2c5d).

Files with missing lines Patch % Lines
ExtractorUtils/Unstable/Runtime/Runtime.cs 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #655      +/-   ##
==========================================
- Coverage   78.07%   77.98%   -0.10%     
==========================================
  Files         136      136              
  Lines       13228    13240      +12     
  Branches     1991     1993       +2     
==========================================
- Hits        10328    10325       -3     
- Misses       2063     2075      +12     
- Partials      837      840       +3     
Files with missing lines Coverage Δ
ExtractorUtils/Unstable/Runtime/Runtime.cs 61.74% <83.33%> (-2.55%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Toshad
Copy link
Copy Markdown
Author

Toshad commented Apr 9, 2026

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new ExtractorRunResult.RestartRequired state to the extractor runtime, allowing for clean shutdowns that still necessitate a restart, particularly when a configuration revision changes. The Run and BuildAndRunExtractor methods have been updated to handle this new state, including logging relevant information and setting appropriate backoff. Unit tests were also updated to reduce backoff times and explicitly set the restart policy to OnError for testing config change scenarios. There are no review comments to address.

Copy link
Copy Markdown
Contributor

@Hmnt39 Hmnt39 left a comment

Choose a reason for hiding this comment

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

Is this restart trigger will only be done on remote config changes or local remote will also do it ?

@Toshad
Copy link
Copy Markdown
Author

Toshad commented Apr 10, 2026

Is this restart trigger will only be done on remote config changes or local remote will also do it ?

If connection config is set to local, then it will do it.

If connection config is set to 'no connection', Then it will not do it.

I think this is correct behaviour.

@Toshad Toshad requested a review from Hmnt39 April 10, 2026 12:06
@Toshad Toshad added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-risk-review Waiting for a member of the risk review team to take an action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants