You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/02/25 16:09:59 UTC
[GitHub] [brooklyn-server] grkvlt commented on a change in pull request #1150: Dependency updates
grkvlt commented on a change in pull request #1150:
URL: https://github.com/apache/brooklyn-server/pull/1150#discussion_r582905757
##########
File path: core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -367,7 +375,7 @@ public URL getResource(String name) {
}
if (bundle == null) {
throw new IllegalStateException("Bundle " + toBundleString(symbolicName, version)
- + " not found to load " + name);
+ + " not found to load " + name + (originalSymbolicName.equals(symbolicName)?"":". Original symbolic name: "+originalSymbolicName));
Review comment:
Suggest moving this code out into the `toBundleString` helper method, where it can append "was originally _originalName_" if it finds `symbolicName` in the rename map
##########
File path: core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -329,9 +332,14 @@ public URL getResource(String name) {
return Maybe.absentNull();
}
- protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher, String symbolicName, String version,
+ protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher, String originalSymbolicName, String version,
String name) {
Framework framework = getFramework();
+ String symbolicName = originalSymbolicName;
+ if(UPDATED_SYMBOLICS_NAMES.containsKey(symbolicName)){
+ log.debug("Using {} as symbolicName instead of {} as it is in UPDATED_SYMBOLICS_NAMES list", symbolicName, originalSymbolicName);
+ symbolicName = UPDATED_SYMBOLICS_NAMES.get(symbolicName);
Review comment:
Instead of setting this here, look for where we use `symbolicName` and do:
```
String nameToSearch = findUpdatedSymbolicName(symbolicName);
```
Reason for this is we may need multiple passes through the map since A becomes B, then B becomes C later, and we may have both A and B, as legacy names, so now need to resolve them both to C.
##########
File path: core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -329,9 +332,14 @@ public URL getResource(String name) {
return Maybe.absentNull();
}
- protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher, String symbolicName, String version,
+ protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher, String originalSymbolicName, String version,
String name) {
Framework framework = getFramework();
+ String symbolicName = originalSymbolicName;
+ if(UPDATED_SYMBOLICS_NAMES.containsKey(symbolicName)){
Review comment:
would prefer a helper method `isSymbolicNameChanged`
##########
File path: core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -76,6 +76,9 @@
private static final String WHITE_LIST_DEFAULT =
"org\\.apache\\.brooklyn\\..*:" + BrooklynVersion.getOsgiVersion();
+ // Contains the bundle symbolic names replaced in new versions mapped to the new name
+ private Map<String,String> UPDATED_SYMBOLICS_NAMES = ImmutableMap.of("org.apache.commons.codec", "org.apache.commons.commons-codec");
Review comment:
Change name of map to `SYMBOLIC_NAME_UPDATES` and use the `ImmutableMap.builder()` syntax to allow for commented entries and future updates.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org