You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "jongyoul (via GitHub)" <gi...@apache.org> on 2026/08/15 06:59:30 UTC
[PR] [HOTFIX] Validate interpreter library lookup [zeppelin]
jongyoul opened a new pull request, #5423:
URL: https://github.com/apache/zeppelin/pull/5423
### What is this PR for?
This PR makes interpreter library lookup more robust and predictable.
The previous lookup used the interpreter setting name directly as the on-disk directory, while dependency repositories are stored by setting ID. The updated lookup resolves the registered interpreter setting first, uses its ID for the repository, and returns only direct, regular JAR entries expected by the library synchronization flow. Missing and unsupported entries are ignored.
This also preserves compatibility when an interpreter setting name differs from its setting ID.
### What type of PR is it?
Hot Fix
### Todos
* [x] Validate interpreter and library lookup inputs
* [x] Resolve repositories through registered interpreter settings
* [x] Preserve setting-name-to-setting-ID compatibility
* [x] Add unit tests for accepted and ignored entries
### What is the Jira issue?
N/A
### How should this be tested?
```bash
./mvnw -pl zeppelin-server \
-Dtest=RemoteInterpreterEventServerTest,RemoteInterpreterEventServerLibraryTest test
./mvnw -pl zeppelin-server -DskipTests \
org.apache.rat:apache-rat-plugin:check -Prat
```
The unit tests cover registered JAR retrieval and metadata, setting-name-to-setting-ID resolution, unsupported lookup inputs, unregistered settings, non-JAR files, directories, and symbolic links.
### 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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HOTFIX] Validate interpreter library lookup [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #5423:
URL: https://github.com/apache/zeppelin/pull/5423#issuecomment-5325911211
Merged into master (8adeb6a4).
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HOTFIX] Validate interpreter library lookup [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul merged PR #5423:
URL: https://github.com/apache/zeppelin/pull/5423
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HOTFIX] Validate interpreter library lookup [zeppelin]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5423:
URL: https://github.com/apache/zeppelin/pull/5423#discussion_r3788803187
##########
zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServer.java:
##########
@@ -584,55 +586,130 @@ public void updateParagraphConfig(String noteId,
@Override
public List<LibraryMetadata> getAllLibraryMetadatas(String interpreter) throws TException {
- if (StringUtils.isBlank(interpreter)) {
- LOGGER.warn("Interpreter is blank");
- return Collections.emptyList();
- }
- File interpreterLocalRepo = new File(
- zConf.getAbsoluteDir(ZeppelinConfiguration.ConfVars.ZEPPELIN_DEP_LOCALREPO)
- + File.separator
- + interpreter);
- if (!interpreterLocalRepo.exists()) {
- LOGGER.warn("Local interpreter repository {} for interpreter {} doesn't exists", interpreterLocalRepo,
- interpreter);
+ Optional<Path> interpreterLocalRepo = resolveInterpreterRepository(interpreter);
+ if (interpreterLocalRepo.isEmpty()) {
+ LOGGER.warn("Unable to resolve local repository for requested interpreter");
return Collections.emptyList();
}
- if (!interpreterLocalRepo.isDirectory()) {
- LOGGER.warn("Local interpreter repository {} is no folder", interpreterLocalRepo);
- return Collections.emptyList();
- }
- Collection<File> files = FileUtils.listFiles(interpreterLocalRepo, new String[] { "jar" }, false);
- List<LibraryMetadata> metaDatas = new ArrayList<>(files.size());
- for (File file : files) {
- try {
- metaDatas.add(new LibraryMetadata(file.getName(), FileUtils.checksumCRC32(file)));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage(), e);
+
+ List<LibraryMetadata> metaDatas = new ArrayList<>();
+ try (DirectoryStream<Path> libraries =
+ Files.newDirectoryStream(interpreterLocalRepo.get(), "*.jar")) {
+ for (Path entry : libraries) {
+ Optional<Path> library = resolveLibrary(interpreter, entry.getFileName().toString());
+ if (library.isPresent()) {
+ try {
+ metaDatas.add(new LibraryMetadata(
+ library.get().getFileName().toString(),
+ FileUtils.checksumCRC32(library.get().toFile())));
+ } catch (IOException e) {
+ LOGGER.warn("Unable to calculate interpreter library checksum", e);
+ }
+ }
}
+ } catch (IOException e) {
+ LOGGER.warn("Unable to list libraries for requested interpreter", e);
}
Review Comment:
`getAllLibraryMetadatas()` iterates `*.jar` entries and then calls `resolveLibrary(interpreter, ...)` for each entry. `resolveLibrary()` re-resolves the interpreter repository (including `toRealPath()` and an `InterpreterSettingManager.getInterpreterSettingByName()` scan) for every file, which is unnecessary work and can become noticeably expensive when an interpreter has many libraries.
Consider validating each `DirectoryStream` entry directly using the already-resolved repository path, so repository resolution happens once per request (not once per JAR).
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org