-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Classgraph to Import Unknown Packages Not Yet Loaded #3395
Conversation
engine/table/build.gradle
Outdated
@@ -52,6 +52,7 @@ dependencies { | |||
// TODO(deephaven-core#3204): t-digest 3.3 appears to have higher errors than 3.2 | |||
implementation 'com.tdunning:t-digest:3.2' | |||
implementation 'com.squareup:javapoet:1.13.0' | |||
implementation 'io.github.classgraph:classgraph:4.8.103' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like classgraph. We need to decide if this additional dependency is worth it. It doesn't have any external dependencies itself, which is nice.
I'd bump to the latest version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've bumped. I'd be interested in any other approach you have in mind. The reported bug is an annoying thing for the user to need to handle.
Maybe we should make groovy-support an extension?
engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one more point.
engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's check w/ @rcaudy too
engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
Show resolved
Hide resolved
I can verify that class graph does not initialize any classes until they are actually loaded: package io.deephaven.server;
public class TestStatic {
static {
System.out.println("Initializing TestStatic");
}
} I ran three commands one at a time, in this order: import io.deephaven.server.barrage.* import io.deephaven.server.* import io.deephaven.server.TestStatic Only the last of which printed my static initializer:
|
Including Ryan's request to avoid initializing the class, this Results in no output: import io.deephaven.server.TestStatic Can finish loading the class like so: println(TestStatic.class.toString()) Yields:
|
Fixes #1129.