Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize Fabric deletion and destroy sessions #251

Merged
merged 11 commits into from
Mar 11, 2023

Conversation

Apollon77
Copy link
Collaborator

When a Fabric gets deleted the current session associated with the Fabric needs to be destroyed. If it is the last fabric all other remaining sessions also needs to be destroyed

@Apollon77 Apollon77 requested a review from mfucci February 12, 2023 21:38
@@ -119,7 +119,13 @@ export const OperationalCredentialsClusterHandler: (conf: OperationalCredentials

Copy link
Owner

Choose a reason for hiding this comment

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

Removing session associated with a fabric when a fabric is deleted should be done by the FabricManager, otherwise there is too much risk to have the system in an incoherent state.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved it to "MatterDevcie" now because FabricManager do not have acces to the session controller but the device is taling care here too ... so should work?

ALternatively Ineed to pass the sessionmanager with removeFabric justto call from there ...

@Apollon77 Apollon77 requested a review from mfucci February 13, 2023 16:19
@Apollon77
Copy link
Collaborator Author

@mfucci COmments addressed ... one topic basically open - see above, but I did a change to where it also could make sense in my eyes. Ready for re-review

Copy link
Owner

@mfucci mfucci left a comment

Choose a reason for hiding this comment

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

Let's make sure the system cannot fall in an incorrect state

@Apollon77
Copy link
Collaborator Author

@mfucci Adjusted as discussed

@Apollon77 Apollon77 requested a review from mfucci March 1, 2023 22:04
@Apollon77 Apollon77 requested a review from mfucci March 7, 2023 09:52
@Apollon77 Apollon77 merged commit 85bdf5e into mfucci:main Mar 11, 2023
@Apollon77 Apollon77 deleted the delete-fabric-finalize branch March 11, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants