estuary-cdk: Add restart_interval
override
#1354
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This allows connectors to override their target lifetime. The problem is that some connectors may have tasks that run for longer than the restart interval. Since the restart signal is soft, i.e all tasks are allowed to run to completion before the connector exits (but no new tasks will be started once the restart signal has been triggered), tasks running longer than the restart interval could effectively starve the shorter-running tasks.
Note: This does feel like kind of a band-aid fix, but I'm not thinking of a better option right now. Needing to restart at all feels like a bit of a smell... so maybe this should be paired with some logic to just never (voluntarily, we should still handle external exit requests properly) restart if not needed?
Also, ideally this would be hidden under an Advanced section, but I wasn't confident enough in my knowledge of Python OOP best practices to define a generic/abstract base
BaseAdvanced
class, and then have the abstractBaseEndpointConfig
include it, while also allowing both to remain extendable by connector authors. If you feel strongly that it should be in Advanced then I'll go and figure this part out, if not I'm happy to leave it as is for now.This change is