move-hoon opened a new pull request, #5289: URL: https://github.com/apache/zeppelin/pull/5289
### What is this PR for? In `NotebookRepoSync`, secondary storage failures were already handled for `save()` and `move()`, but `remove()` still used a generic `for` loop with a `TODO` comment for handling secondary remove failures. This PR makes the `remove()` failure handling consistent with the existing `save()` and `move()` behavior: * **Primary Storage (`getRepo(0)`)**: A failure during removal stops the operation and propagates the `IOException`. * **Secondary Storage (`getRepo(1)`)**: A failure during removal is caught and explicitly logged, preventing silent inconsistencies without interrupting the overall operation. To achieve this, the `for` loop in `remove(String noteId, String notePath, AuthenticationInfo subject)` was replaced with explicit `getRepo(0)` and `getRepo(1)` calls, and the outstanding `TODO` was removed. ### What type of PR is it? Improvement ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-6453 ### How should this be tested? Tested via `NotebookRepoSyncTest` using Mockito to explicitly verify the primary vs. secondary failure behaviors: * `testRemoveSucceedsWhenSecondaryRepoFails`: Verifies that an `IOException` from the secondary repo is caught and does not stop the removal process. * `testRemoveFailsWhenPrimaryRepoFails`: Verifies that an `IOException` from the primary repo halts the operation and propagates the exception. * All existing tests in `NotebookRepoSyncTest` continue to pass. ### Screenshots (if appropriate) N/A ### Questions: * Does the license files need to update? No * Is there breaking changes for older versions? No * Does this needs documentation? No -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
