fix(QTDI-2004): check if running into jobserver #1105
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| this.logInfoLevelMapping = logInfoLevelMapping; | ||
| this.containerInitializer = containerInitializer; | ||
| this.resolver = dependenciesResolutionConfiguration.getResolver(); | ||
| final boolean hasClasspathJar = "classpath.jar".equals(System.getProperty("java.class.path", "")); |
There was a problem hiding this comment.
"classpath.jar" is valid for remote job server run way and other cases? in my memory, is only for local run in studio which as test mode for customer. Maybe i am wrong.
There was a problem hiding this comment.
https://qlik-dev.atlassian.net/browse/SUPPORT-6134
so we can't suppose classpath.jar for jobserver/remote engine case.
There was a problem hiding this comment.
Correct me if I'm wrong but it seems that the classpath.jar is alway created by the JobServer when it receives a job execution to avoid artifacts duplications.
local run of a studio job exported to zip, uses a batch script to launch job's execution. This script sets the m2 system property used by component manager to the lib folder provided by export.
java -Dtalend.component.manager.m2.repository=$ROOT_PATH/../lib ...
Thus not the specific case related.
In fact, this fix apply only when a classpath.jar is provided to jvm, not the other cases when the m2 property is correctly set.
The SUPPORT-6134 issue mentions "Cloud Export" and specifies in the description Talend > Import/Export > Add classpath jar in exported jobs (not compatible with JobServer).
Apparently, Cloud export works the same as local exported jobs...
There was a problem hiding this comment.
Added case when CLASSPATH_SIZE_THRESHOLD (in JobServer properties) value is set to a very high value. Thus, no classpath.jar is generated.
- remove `nested:` for embedded m2 repository
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (nested != null) { | ||
| rootRepo = PathFactory.get(nested.getFile().replace("file:", "")); | ||
| info("Nested maven repository: " + nested.getFile()); | ||
| rootRepo = PathFactory.get(nested.getFile().replaceAll("^(file|nested):", "")); |
There was a problem hiding this comment.
The regex pattern ^(file|nested): uses a hardcoded string that may not cover all URL scheme variations. Consider using URL.toURI().getPath() or a more robust URL parsing approach to handle different schemes consistently.
| rootRepo = PathFactory.get(nested.getFile().replaceAll("^(file|nested):", "")); | |
| try { | |
| rootRepo = PathFactory.get(nested.toURI().getPath()); | |
| } catch (Exception e) { | |
| throw new IllegalStateException("Failed to extract path from nested repository URL: " + nested, e); | |
| } |
Copilot uses AI. Check for mistakes.
| this.logInfoLevelMapping = logInfoLevelMapping; | ||
| this.containerInitializer = containerInitializer; | ||
| this.resolver = dependenciesResolutionConfiguration.getResolver(); | ||
| final boolean hasClasspathJar = "classpath.jar".equals(System.getProperty("java.class.path", "")); |
There was a problem hiding this comment.
The hardcoded string classpath.jar should be extracted as a constant to improve maintainability and make the detection logic more explicit.
Copilot uses AI. Check for mistakes.
| container.getDefinedNestedPlugin().stream().filter(p -> !hasPlugin(p))) | ||
| .forEach(this::addPlugin); | ||
| info("Components: " + availablePlugins()); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
seems here only catch the exception and ignore it, is enough?
https://qlik-dev.atlassian.net/browse/QTDI-2004