> // see https://issues.apache.org/jira/browse/JCLOUDS-570 > Optional<? extends Image> image = tryFind(images, idPredicate); > - if (!image.isPresent()) { > - logger.warn("Image %s not found in the image cache. Trying to get > it directly...", imageId); > - // Note that this might generate make a call to the provider > instead of using a cache, but > - // this will be executed rarely, only when an image is not present > in the image list but > - // it actually exists in the provider. It shouldn't be an expensive > call so using a cache just for > - // this corner case is overkill. > - image = Optional.fromNullable(getImageStrategy.getImage(imageId)); > - if (!image.isPresent()) { > - throwNoSuchElementExceptionAfterLoggingImageIds(format("%s not > found", idPredicate), images); > - } > + if (image.isPresent()) { > + return image.get(); > + } > + > + logger.warn("Image %s not found in the image cache. Trying to get it > from the provider...", imageId);
There needs to be another log message after the image has been fetched confirming that it was fetched and cached. If I saw only the message above in a log, it would look like an error to me because it's "Trying..." but never succeeds? Or you could just move the log message to after the image fetch and change the message to be in the past tense. e.g. ""Image %s not found in the image cache. Image was fetch from the provider and cached in jclouds." Also, should this be a debug statement? It's not exactly unexpected behaviour. I'm not sure. I could go either way. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/401/files#r13655674
