feat: terminate data flow on transfer completion #3481
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3481 +/- ##
==========================================
+ Coverage 72.08% 72.13% +0.05%
==========================================
Files 841 839 -2
Lines 16996 17036 +40
Branches 951 955 +4
==========================================
+ Hits 12251 12289 +38
- Misses 4341 4345 +4
+ Partials 404 402 -2
☔ View full report in Codecov by Sentry. |
ndr-brt
changed the title
ndr-brt
force-pushed
the
3453-stop-dataflow-on-completion
branch
from
91a6425
to
94909ca
Compare
ndr-brt
changed the title
ndr-brt
requested review from
paullatzelsperger,
bscholtes1A and
jimmarino
as code owners
ndr-brt
force-pushed
the
3453-stop-dataflow-on-completion
branch
2 times, most recently
from
5ebe8cd
to
1f25856
Compare
|
@ndr-brt It would be good to first add a DR outlining this new state and its uses. |
There was a problem hiding this comment.
I had a few questions regarding "terminating" vs "stopping".
- when we
terminate, we send aTransferTerminationMessage, but when westop, we don't send anything. Is that on purpose? Shouldn't we notify the counter-party, that a transfer has been stopped? - conversely, when we "terminate", we don't invoke the dataflow manager, so theoretically there could still be an ongoing transfer, right?
- isn't "stopping" almost the same as "terminating", but with a "configurable" next state? Or rather: isn't terminating just stopping with a preconfigured next state?
If those questions are scoped too broadly, we can just merge the PR and discuss elsewhere
| public void transitionStopping(String reason, TransferProcessStates subsequentState) { | ||
| transition(STOPPING, STARTED); | ||
| errorDetail = reason; | ||
| privateProperties.put("stopping-subsequent-state", subsequentState); |
| @@ -91,6 +92,11 @@ public ServiceResult<Stream<TransferProcess>> query(QuerySpec query) { | |||
| }); | |||
| } | |||
|
|
|||
| @Override | |||
| public ServiceResult<Void> stop(StopTransferCommand command) { | |||
There was a problem hiding this comment.
to keep consistency with the other methods, this should probably also be @NotNull
ndr-brt
force-pushed
the
3453-stop-dataflow-on-completion
branch
from
1f25856
to
6267be7
Compare
|
@paullatzelsperger , yes, sorry, I discussed with @jimmarino after then and decided to avoid that STOPPING status, instead the data flow termination will be invoked before the transfer process termination. I updated the PR accordingly, now it's ready to be reviewed. |
ndr-brt
force-pushed
the
3453-stop-dataflow-on-completion
branch
from
6267be7
to
7544346
Compare
ndr-brt
force-pushed
the
3453-stop-dataflow-on-completion
branch
2 times, most recently
from
37919df
to
f8a509f
Compare
ndr-brt
force-pushed
the
3453-stop-dataflow-on-completion
branch
from
f8a509f
to
960e5a2
Compare
wolf4ood
approved these changes
Oct 4, 2023
jimmarino
approved these changes
Oct 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR changes/adds
Ensures that the data flow gets terminated when the PolicyMonitor detects a no more valid policy.
Why it does that
policy monitor
Further notes
DataFlowControllercan be terminated through theterminatemethodcloseon theDataSourceLinked Issue(s)
Closes #3453
Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.