PR:

I originally changed it to use an Optional, but as I was updating the consumers, every update ended up using .orElseThrow() since other StorageExceptions were expected within the same code blocks.

I forgot to add a comment here, but I discussed this with Chandra and my plan is to change the interface to return an Optional and adjust consumer behavior, so essentially option 2.

you asked about whether we can close this ticket. Although we are in the process of removing one of the cases that can trigger it with , the bug still exists and can be triggered by misconfiguration and other runtime problems.

Anytime this else statement is reached …

… get() will return a null and there are still a pile of consumers that won’t check for that case and will get a NullPointerException. We fixed one of the consumers in but there are still half a dozen others.

I think the key thing is to clarify whether implementations of TimeseriesStorageManager.get() can return nulls and based on the answer, I think there are (at least) two paths to go down:

1. If we decide nulls are not allowed,

   1. Add a comment to the interface that a null will never be returned (or is there an annotation to reflect this?), and

   2. Change TimeseriesStorageManagerImpl.get() to do either of these:

      1. Return something like a default or no-op TimeseriesStorage implementation, or

      2. Throw an exception if there is no TimeseriesStorage implementation available (including similar details to the existing warning log message).

2. If we decide nulls are allowed,

   1. Mark the interface as @Nullable, and

   2. Fix all of the consumers to do the appropriate thing if a null is returned.

I think 1 is definitely easier, but this type of interface design decision is not a strength of mine, so don’t take my word on which way to go.