feat: STREAMP-8526, STREAMP-10704: Support zone and infrastructure deletion #356
Conversation
| .findAll(pb -> pb.getKey().getInfrastructureName().equals(infrastructure.getKey().getName())) | ||
| .findAny() | ||
| .ifPresent(pb -> { throw new IllegalStateException("Infrastructure is used in producer binding: " + pb.getKey()); }); | ||
|
|
There was a problem hiding this comment.
There is no reference of infrastructure in process binding.
Do we need to check using zone?
processBindingView
.findAll(pb -> pb.getKey().getInfrastructureZone().equals(infrastructure.getKey().getZone()))
.findAny()
.ifPresent(pb -> { throw new IllegalStateException("Infrastructure is used in process binding: " + pb.getKey()); });
There was a problem hiding this comment.
You are right, the ProcessBinding -> Infrastructure reference is indirect via the StreamBinding on inputs and outputs.
I feel like we should be as defensive as possible and still do the check on those inputs/outputs.
There was a problem hiding this comment.
Additionally look here. It's currently possible to delete a StreamBinding when there is a reference it from a ProcessBinding. This could lead to dodgy data and orphaned entities.
|
We need to update InfrastructureTestStage and |
| @@ -97,6 +99,12 @@ public void upsert() { | |||
| @Override | |||
| public void delete() { | |||
| //not implemented for infrastructure | |||
There was a problem hiding this comment.
This needs removing 😄
There was a problem hiding this comment.
This is now removed 👍
|
|
||
| private boolean isZoneUsedInProcessBindingOutput(Zone zone, ProcessBinding processBinding) { | ||
| return processBinding.getOutputs().stream().map(o -> o.getStreamBindingKey().getInfrastructureZone()).toList() | ||
| .contains(zone.getKey().getName()); |
There was a problem hiding this comment.
Is there any need for the toList.contains()? I think we can use anyMatch()?
There was a problem hiding this comment.
Yes, it's not required to convert to list unnecessarily. I'll replace it with anyMatch()
| throw new UnsupportedOperationException("Infrastructure deletion not currently supported."); | ||
| handlerService.handleDelete(infrastructure); | ||
| streamBindingView | ||
| .findAll(sb -> sb.getKey().getInfrastructureName().equals(infrastructure.getKey().getName())) |
There was a problem hiding this comment.
Third time looking at this PR and just noticed, shouldn't this be comparing on the InfrastructureKey, not the Infrastructure name?
| .findAll(sb -> sb.getKey().getInfrastructureName().equals(infrastructure.getKey().getName())) | |
| .findAll(sb -> sb.getKey().getInfrastructureKey().equals(infrastructure.getKey())) |
There was a problem hiding this comment.
The name and the zone together identify an Infra
stream-registry PR
<High level description of the PR>
Added
Changed
Deleted
PR Checklist Forms