renechoi commented on code in PR #4977:
URL: https://github.com/apache/zeppelin/pull/4977#discussion_r2232359309
##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java:
##########
@@ -309,9 +313,14 @@ private void loadFromFile() throws IOException {
}
if (infoSaving.interpreterRepositories != null) {
- for (RemoteRepository repo : infoSaving.interpreterRepositories) {
- if (!dependencyResolver.getRepos().contains(repo)) {
+ for (Repository repo : infoSaving.interpreterRepositories) {
+ // Check if repository already exists to avoid duplicates
+ boolean exists = this.interpreterRepositories.stream()
+ .anyMatch(r -> r.getId().equals(repo.getId()));
+ if (!exists) {
Review Comment:
@Reamer Thank you for the feedback!
I've updated the code as requested:
- When loading from file, existing repositories with the same ID are now
removed and overwritten (last one wins)
- The `addRepository()` method already had this behavior, so now both file
loading and API calls behave consistently
- Tests have been updated to reflect the new behavior
@ankursaini2006 Thanks for catching this inconsistency! The behavior is now
unified between file loading and API calls.
I agree that refactoring to use `Map<String, Repository>` would be a good
improvement for a follow-up PR.
--
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]
