ctubbsii commented on code in PR #35:
URL:
https://github.com/apache/accumulo-classloaders/pull/35#discussion_r2677434493
##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java:
##########
@@ -206,70 +121,168 @@ void resetForTests() {
@Override
public void init(ContextClassLoaderEnvironment env) {
- baseCacheDir =
requireNonNull(env.getConfiguration().get(Constants.CACHE_DIR_PROPERTY),
+ String value =
requireNonNull(env.getConfiguration().get(Constants.CACHE_DIR_PROPERTY),
"Property " + Constants.CACHE_DIR_PROPERTY + " not set, cannot create
cache directory.");
String graceProp =
env.getConfiguration().get(Constants.UPDATE_FAILURE_GRACE_PERIOD_MINS);
long graceMins = graceProp == null ? 0 : Long.parseLong(graceProp);
updateFailureGracePeriodMins = Duration.ofMinutes(graceMins);
+ final Path baseCacheDir;
+ if (value.startsWith("file:")) {
+ try {
+ baseCacheDir = Path.of(new URL(value).toURI());
+ } catch (IOException | URISyntaxException e) {
+ throw new IllegalArgumentException(
+ "Malformed file: URL specified for base directory: " + value, e);
+ }
+ } else if (value.startsWith("/")) {
+ baseCacheDir = Path.of(value);
+ } else {
+ throw new IllegalArgumentException(
+ "Base directory is neither a file URL nor an absolute file path: " +
value);
+ }
try {
Review Comment:
If we check it, that check could be invalid moments later. We're going to
fail pretty fast if we can't create the contexts and resources directories
right away anyway, so there's not much reason to check this.
--
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]