Skip to content

Conversation

@undx
Copy link
Member

@undx undx commented Oct 29, 2025

https://qlik-dev.atlassian.net/browse/QTDI-2031

This pull request mainly

  • reverts changes in QTDI-693
  • adds some extra debug logging informations
  • add annotation for marking a dynamic dependencies configuration

Related pull-requests:

@undx undx requested a review from Copilot October 29, 2025 14:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reverts changes from QTDI-693 related to nested repository handling and adds debugging capabilities for dynamic dependencies troubleshooting. The changes simplify classloader behavior by removing nested URL tracking and redundant scanning logic.

Key Changes:

  • Removes nested repository special handling and scanning logic that was added in QTDI-693
  • Adds debug logging for system information (JVM arguments, classpaths, repository locations)
  • Introduces @DynamicDependenciesConfiguration annotation for marking dynamic dependency configurations

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ContainerManager.java Adds debug logging for system info and repository locations; simplifies root repository resolution
ConfigurableClassLoader.java Removes nested URL tracking by deleting nestedURLs field and getURLs() override
ComponentManager.java Removes nested repository scanning logic and unused imports
ComponentManagerTest.java Updates test assertions to reflect reverted behavior (expects 1 dependency instead of 2)
DynamicDependenciesConfiguration.java New annotation file for marking dynamic dependencies configuration classes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

if (nested != null) {
rootRepo = PathFactory.get(nested.getFile().replace("file:", ""));
}
.orElseGet(() -> PathFactory.get(System.getProperty("user.home", "")).resolve(".m2/repository"));
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

Using an empty string as the default value for 'user.home' system property will cause PathFactory.get(\"\") to be called if the property is not set, which may result in unexpected path resolution behavior. Consider using a more appropriate fallback or handling the null case explicitly.

Suggested change
.orElseGet(() -> PathFactory.get(System.getProperty("user.home", "")).resolve(".m2/repository"));
.orElseGet(() -> {
String userHome = System.getProperty("user.home");
if (userHome == null || userHome.isEmpty()) {
userHome = System.getProperty("user.dir");
if (userHome == null || userHome.isEmpty()) {
throw new IllegalStateException("Neither 'user.home' nor 'user.dir' system properties are set. Cannot determine repository location.");
}
}
return PathFactory.get(userHome).resolve(".m2/repository");
});

Copilot uses AI. Check for mistakes.
@undx undx changed the title fix(QTDI-2031): dynamic dependencies fix(QTDI-2031): manage dynamic dependencies - w/o MAVEN-INF Oct 30, 2025
@sonar-eks
Copy link

sonar-eks bot commented Oct 30, 2025

Failed Quality Gate failed

  • 55.20% Coverage on New Code (is less than 80.00%)
  • 2 New Issues (is greater than 0)

Project ID: org.talend.sdk.component:component-runtime

View in SonarQube

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