feat: DH-20578: Groovy remote file sourcing by bmingles · Pull Request #7451 · deephaven/deephaven-core

bmingles · 2025-11-26T22:40:53Z

DH-20578: This PR implements a RemoteFileSource plugin in Deephaven Core that can request file sources from a remote client. Key points of interest

RemoteFileSourceClassLoader.java - registered with the Groovy session to request remote file sources
RemoteFileSourceCommandResolver.java - resolves a flight command to an instance of the plugin service. This allows us to not have to register a magic variable in the session. It exports a PluginMarker object that is associated with a plugin name that the RemoteFileSourcePlugin later recognizes as an object it can suppport
RemoteFileSourcePlugin - the plugin
PluginMarker - named object used to match the flight command with the plugin. Needed since all plugins require an exported object of some sort
GroovyDeephavenSession.java - clearing class cache when remote sources are involved

Related PRs

deephaven/vscode-deephaven/pull/273 implements the client side code to be used by the VS Code extension.
deephaven-ent/iris/pull/3878 is needed to enable this in Core+

Testing

Install the client side code in VS Code vscode-deephaven-1.0.18-remote-file-source.1.vsix

Community

Run Core Groovy server from this branch (I use IntelliJ debug config :web-client-api:gwtCompile server-jetty-app:run -Panonymous -Pgroovy)
Clone this repo https://github.com/bmingles/deephaven-groovy-remote-docs-demo and open the folder in VS Code
In the Deephaven view, expand the Workspace/Groovy/groovy-remote-docs-demo folder under REMOTE IMPORT SOURCES and click the + button to add test folder.
Open docs-sample.groovy file and run against the Deephaven server. Should see testTable panel show up.

Core+

Changes are deployed to bmingles-remote-file-source vm. Can also configure VS Code to connect to https://bmingles-remote-file-source.int.illumon.com:8123/ and run same code

github-actions · 2025-11-26T22:41:04Z

No docs changes detected for `9bcc8b2`

bmingles · 2026-01-02T21:50:11Z

    implementation project(':server-jetty-11')

    implementation project(':extensions-flight-sql')
+    implementation project(':plugin-remotefilesource')


Not sure if these should have been added here since I don't see any other plugins.

bmingles · 2026-01-02T21:50:18Z

    implementation project(':server-jetty')

    implementation project(':extensions-flight-sql')
+    implementation project(':plugin-remotefilesource')


Not sure if these should have been added here since I don't see any other plugins.

niloc132 · 2026-01-06T15:52:15Z

+/**
+ * Utility methods for working with protobuf messages in the client-side JavaScript environment.
+ */
+public class JsProtobufUtils {


...how sure are we that we actually need any/all of this? doesnt the generated message types give us this for free?

I thought we needed it in order to support the Any wrapper?

niloc132 · 2026-01-06T15:52:49Z

+@JsType(namespace = "dh.remotefilesource", name = "RemoteFileSourceService")
+public class JsRemoteFileSourceService extends HasEventHandling {
+    /** Event name for generic messages from the server */
+    @JsProperty(namespace = "dh.remotefilesource.RemoteFileSourceService")


this annotation shouldnt be necessary, should get it from the type's namespace+name, ditto below

I got warnings for the @link #EVENT_MESSAGE in the header comment. Claude suggested this fix which worked. Is there a better way?

By adding hte JsType annotation at the class top level, all public static constants are added to the JS automatically. See JsPartitionedTable for some precedent.

@mofojed This produced warnings for the @link #EVENT_MESSAGE syntax in header comments for me

Where are you seeing this warning? I'm not seeing any warning in the header or anywhere else in the file related to this when I remove this annotation (the @JsProperty annotation)

@mofojed
If I remove the annotation and build the JS API types via ./gradlew :web-client-api:types:assemble

I get:

[warning] Failed to resolve link to "dh.remotefilesource.EVENT_REQUEST_SOURCE" in comment for dh.remotefilesource.RemoteFileSourceService.

Although, there are a number of other warnings as well. I just didn't want to keep adding to them:

[warning] Failed to resolve link to "dh.convertArrayToString" in comment for dh.PreviewOptions. [warning] Failed to resolve link to "dh.array" in comment for dh.PreviewOptions. [warning] Failed to resolve link to "dh.transportFactory" in comment for dh.ConnectOptions.useWebsockets. [warning] Failed to resolve link to "dh.useWebsockets" in comment for dh.ConnectOptions.transportFactory. [warning] Failed to resolve link to "Table.uncoalesced" in comment for dh.Column.isPartitionColumn. [warning] Failed to resolve link to "Promise" in comment for dh.IdeConnection.mergeTables. [warning] Failed to resolve link to "io.deephaven.web.shared.fu.JsRunnable" in comment for dh.IdeConnection.onLogMessage. [warning] Failed to resolve link to "io.deephaven.web.shared.fu.JsRunnable" in comment for dh.IdeConnection.onLogMessage.__type. [warning] Failed to resolve link to "dh.includeConstituents" in comment for dh.TreeTable. [warning] Failed to resolve link to "TreeRowImpl.hasChildren" in comment for dh.TreeTable. [warning] Failed to resolve link to "java.lang.UnsupportedOperationException" in comment for dh.TreeTable.applyCustomColumns. [warning] Failed to resolve link to "io.deephaven.engine.table.PartitionedTable.merge" in comment for dh.PartitionedTable.getMergedTable. [warning] Failed to resolve link to "DataOptions.SubscriptionOptions" in comment for dh.Table.subscribe. [warning] Failed to resolve link to "io.deephaven.web.client.api.subscription.AbstractTableSubscription.close" in comment for dh.Table.createSubscription. [warning] Failed to resolve link to "Table.uncoalesced" in comment for dh.Table.size. [warning] Failed to resolve link to "Promise" in comment for dh.IdeSession.getTreeTable. [warning] Failed to resolve link to "Promise" in comment for dh.IdeSession.mergeTables. [warning] Failed to resolve link to "Promise" in comment for dh.IdeSession.emptyTable. [warning] Failed to resolve link to "Promise" in comment for dh.IdeSession.timeTable. [warning] Failed to resolve link to "JsTable#createSnapshot(Object)" in comment for dh.TableViewportSubscription.snapshot. [warning] Failed to resolve link to "dh.remotefilesource.EVENT_REQUEST_SOURCE" in comment for dh.remotefilesource.RemoteFileSourceService.

niloc132 · 2026-01-06T16:28:45Z

+                    "RemoteFileSourcePluginFetchRequest must contain a valid result_id");
+        }
+
+        final String pluginName = request.getPluginName();


it appears that DeephavenRemoteFileSourcePlugin is always the pluginName set by the client, so we end up with exactly one PluginMarker. Do we anticipate more of these, or specific clients who can control this? Nothing seems able to clear old instances, do we want a mechanism (liveness?) for that?

I anticipate that the Python version of the plugin may make use of this at some point.

Clearing instances is probably a good idea. Any suggestions on best way to do that?

mofojed

Functionality seems good overall, nice.

mofojed · 2026-01-12T16:21:10Z

+            throw new IllegalArgumentException("messageStream must not be null");
+        }
+
+        executionContext = new RemoteFileSourceExecutionContext(messageStream, resourcePaths);


Should we throw if the executionContext is already set?

I don't think so. executionContext is static and only 1 MessageStream instance can claim it at a time. This probably doesn't impact Core+, but in Community it could matter if you had multiple connections to the worker.

Added a better block comment to the method.

mofojed · 2026-01-12T16:33:10Z

+@JsType(namespace = "dh.remotefilesource", name = "RemoteFileSourceService")
+public class JsRemoteFileSourceService extends HasEventHandling {
+    /** Event name for generic messages from the server */
+    @JsProperty(namespace = "dh.remotefilesource.RemoteFileSourceService")


By adding hte JsType annotation at the class top level, all public static constants are added to the JS automatically. See JsPartitionedTable for some precedent.

mofojed · 2026-01-12T16:34:26Z

+        DomGlobal.setTimeout(ignore -> fireEvent(EVENT_REQUEST_SOURCE,
+                new ResourceRequestEvent(message.getRequestId(), request)), 0);


Comment why this is adding a timeout here instead of just calling it right away.

I'm not actually 100% sure why this is needed except that I saw precedence in JsWidget for dispatching new events in response to message stream events. I assume it's desirable to ensure this happens on the next tick, but I can't speak to a specific problem it may cause if we don't do so.

Looking at the history in JsWidget, looks like it was added with this PR: #6715
In that case, I think the issue was the promise for fetching the widget would resolve after we had already fired off the events, so client didn't have time to add a listener to actually receive the event. So just put emitting the event on the next tick after resolving.
I guess in theory that might be possible here as well... though maybe we should be checking if there's no listeners and throwing in that case, or force a listener to be passed in when retrieving the plugin. When there's a request from the server, it's blocked until there's a response, yeah? So we must have a listener to respond here (or we should default to a no-op response, but then why bother fetching the plugin...), and it doesn't make sense to have multiple listeners registered (since multiple responses for one request is invalid), so really we want exactly one listener... may as well enforce it with creation of the service?
In CoreClient#getRemoteFileSourceService(), we're returning a new instance of the service plugin whenever that method is called - what happens if there's multiple instances? We really only want one instance, right?

I made 2 primary changes to address this:

CoreClient#getRemoteFileSourceService() now creates a single instance of JsRemoteFileSourceService

JsRemoteFileSourceService enforces that only 1 EVENT_REQUEST_SOURCE handler can be registered. It also throws if a request for source is received and there are no registered event handlers

Also removed the setTimeout

mofojed

Changes look good, had some questions about fetching an instance of the service and how we handle multiple instances.

mofojed

Marking as needing more changes, as I think this has a caching issue right now? Maybe add a comment on the PR about what the issue is, and the plan to fix it. Let me know if you need any help figuring it out.

bmingles · 2026-01-22T16:18:35Z

There seems to be an issue with GroovyClassLoader caching previously run scripts such that changes after initial run are not picked up. Next step is to confirm that it is the GroovyClassLoader that is doing the caching.

mofojed · 2026-02-27T18:10:12Z

+        // opportunity to be remotely fetched. If remote sources aren't involved, no need to clear the cache.
+        if (previousEvalHadRemoteSources || hasRemoteSources) {
+            log.debug("Remote file sourcing enabled or was previously enabled. Clearing class cache.");
+            refreshClassLoader();


How much of a performance hit is this? Could we improve this by having the plugin notify when changes have been made and only clearing the cache then?

@mofojed I'd need to think through how to implement that, but wouldn't it be safe to assume that the majority of cases where a user re-runs a script would imply changes have been made to a source script? Like even a small change to 1 line requires the full cache eviction, so not sure we'd prevent clearing the cache very often.

You're running a file from a script; you can make a ton of changes to that/re-run and not change any of your imported modules for quite a while. Only need to evict the cache after you make changes to those imported modules AND re-run the script.

gotcha. I wonder then if the execution context can track an isDirty flag that is true if

sources are added / removed

sources in the context are changed

@mofojed I've made the caching improvement. Should only evict cache if remote source list changes or if a dependency changes. Won't evict if only the entry point file changes.

mofojed · 2026-02-27T18:19:15Z

+        GroovyClassLoader classLoader = new GroovyClassLoader(STATIC_LOADER, config);
+
+        // Update the session's ExecutionContext with fresh QueryCompiler
+        executionContext = createExecutionContext(


Instead of building a whole new ExecutionContext, could we just add a setQueryCompiler method on the ExecutionContext and update that directly? Is there precedent for replacing the ExecutionContext somewhere?

I don't see any precedent for replacing the execution context, but I also don't see precedent for any setXxx methods on ExecutionContext. There is a builder pattern and a handful of withXxx methods used to create copies with 1 property modification. I ended up adding a withQueryCompiler method to at least encapsulate more of the code.

Okay, I like the with way better, but would like confirmation from @cpwright or @rcaudy that this won't cause other issues... I imagine in most places we should be calling .getContext() instead of retaining a reference to the context, but wondering if there's some place they may be cached locally where this would cause unexpected results? Like an updating table or something?

mofojed · 2026-02-27T18:58:23Z

+// Server → Client: Requests sent from server to client via MessageStream
+message RemoteFileSourceServerRequest {
+  // Unique identifier for this request, used to correlate the response
+  string request_id = 1;
+
+  oneof request {
+    // Request source data/resource from the client
+    RemoteFileSourceMetaRequest meta_request = 2;
+
+    // Acknowledgment that execution context was set
+    SetExecutionContextResponse set_execution_context_response = 3;
+  }
+}
+
+// Client → Server: Requests/responses sent from client to server via MessageStream
+message RemoteFileSourceClientRequest {
+  // The request_id from the ServerRequest this is responding to (if applicable)
+  string request_id = 1;
+
+  oneof request {
+    // Response to a resource request
+    RemoteFileSourceMetaResponse meta_response = 2;
+
+    // Set the execution context for script execution
+    SetExecutionContextRequest set_execution_context = 3;
+  }
+}


This is confusing - The outer message is Request but it is both requests and responses.
I'd probably name them RemoteFileSourceServerMessage and RemoteFileSourceClientMessage instead.

I've renamed these. Note that there is a gwt wrapper generation step that Colin has to do. I did a find / replace change and things seem to work, but there's a chance a regeneration might produce additional changes.

…e and RemoteFileSourceServerRequest -> RemoteFileSourceServerMessage (#DH-20578)

feat: DH-20578: regen gwt bindings

niloc132 · 2026-03-06T16:50:21Z

Please run ./gradlew updateProtobuf, should get the build passing.

mofojed

Most of my comments have been addressed, but still would like review on this comment: #7451 (comment)
Specifically, we call ExecutionContext#open() in many, many places. That's a public API; how can we be sure that we can replace the execution context, and any consumer of the execution context is up to date? I don't think we can guarantee that, so this change would almost certainly break something. E.g. EmbeddedServer, opens the context permanently; then we're just blowing it away. Those are very difficult to debug problems.
I see ExecutionContext still has a getQueryCompiler() method there so theoretically someone could still cache that, but that's not a) a resource that you open/close, and b) smaller footprint... so I think that would be safer? Would really prefer someone more knowledgeable to provide review there.

niloc132 · 2026-03-16T20:49:30Z

Most of my comments have been addressed, but still would like review on this comment: #7451 (comment) Specifically, we call ExecutionContext#open() in many, many places. That's a public API; how can we be sure that we can replace the execution context, and any consumer of the execution context is up to date? I don't think we can guarantee that, so this change would almost certainly break something. E.g. EmbeddedServer, opens the context permanently; then we're just blowing it away. Those are very difficult to debug problems. I see ExecutionContext still has a getQueryCompiler() method there so theoretically someone could still cache that, but that's not a) a resource that you open/close, and b) smaller footprint... so I think that would be safer? Would really prefer someone more knowledgeable to provide review there.

This definitely could break open(), but EmbeddedServer is a non-issue (as Brian pointed out to me), as EmbeddedServer exists just for running from python, and we don't reaaaally support running Groovy from Python.

Mutating ExecContext seems far more likely to be bad news than changing the execcontext that the script session uses.

deephaven-core/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java

Lines 164 to 168 in f2a9411

    
           // Actually evaluate the script; use the enclosing auth context, since AbstractScriptSession's 
        
           // ExecutionContext never has a non-null AuthContext 
        
           executionContext.withAuthContext(ExecutionContext.getContext().getAuthContext()) 
        
                   .withQueryScope(queryScope) 
        
                   .apply(() -> evaluate(script, scriptName));

That's the only "open" that really matters here, and that will close when the current block of code is finished executing.

Copilot

Pull request overview

Implements “Groovy remote file sourcing” (DH-20578) by adding a server-side plugin + classloader integration and the corresponding client/proto bindings so Groovy scripts can import .groovy sources supplied by a remote client over a message stream.

Changes:

Adds plugin-remotefilesource (Flight command resolver + ObjectType plugin + message stream provider) and integrates it into server runtime.
Introduces RemoteFileSourceClassLoader / provider interface and updates Groovy session execution to refresh compilation cache when remote sources are dirty/removed.
Adds new remotefilesource.proto plus generated bindings for JS (client-backplane + raw-js-openapi), Python, and Go; adds a JS client API service for remote file source interactions.

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/remotefilesourceservermessage/RequestCase.java	New JS interop enum wrapper for server-message oneof case.
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/remotefilesourceclientmessage/RequestCase.java	New JS interop enum wrapper for client-message oneof case.
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/SetExecutionContextResponse.java	Generated JS interop type for `SetExecutionContextResponse`.
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/SetExecutionContextRequest.java	Generated JS interop type for `SetExecutionContextRequest`.
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/RemoteFileSourceServerMessage.java	Generated JS interop type for server→client message stream proto.
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/RemoteFileSourcePluginFetchRequest.java	Generated JS interop type for Flight “fetch plugin” command proto.
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/RemoteFileSourceMetaResponse.java	Generated JS interop type for meta response (content/found/error).
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/RemoteFileSourceMetaRequest.java	Generated JS interop type for meta request (resource name).
web/client-backplane/src/main/java/io/deephaven/javascript/proto/dhinternal/io/deephaven_core/proto/remotefilesource_pb/RemoteFileSourceClientMessage.java	Generated JS interop type for client→server message stream proto.
web/client-api/src/main/java/io/deephaven/web/client/api/remotefilesource/ResourceContentUnion.java	TS/JS union type for resource content (String or Uint8Array).
web/client-api/src/main/java/io/deephaven/web/client/api/remotefilesource/JsRemoteFileSourceService.java	JS client service to fetch plugin instance and handle message stream requests/responses.
web/client-api/src/main/java/io/deephaven/web/client/api/JsProtobufUtils.java	Adds client-side helper to wrap messages into `google.protobuf.Any` bytes.
web/client-api/src/main/java/io/deephaven/web/client/api/CoreClient.java	Exposes a cached `getRemoteFileSourceService()` accessor.
settings.gradle	Adds `:plugin-remotefilesource` project inclusion.
server/jetty-app/build.gradle	Adds remote file source plugin to runtime classpath.
server/jetty-app-11/build.gradle	Adds remote file source plugin to runtime classpath (Jetty 11).
server/build.gradle	Adds remote file source plugin to server runtime dependencies.
py/client/deephaven_core/proto/remotefilesource_pb2_grpc.py	Generated Python gRPC stubs for new proto.
py/client/deephaven_core/proto/remotefilesource_pb2.pyi	Generated mypy-protobuf typing stubs for new proto.
py/client/deephaven_core/proto/remotefilesource_pb2.py	Generated Python protobuf message definitions for new proto.
proto/raw-js-openapi/webpack.config.js	Adds `remotefilesource` to JS shim alias generation list.
proto/raw-js-openapi/src/shim/remotefilesource_pb.js	Adds shim export for JS raw-openapi bundle.
proto/raw-js-openapi/src/index.js	Exports `remotefilesource_pb` in raw-js-openapi index.
proto/proto-backplane-grpc/src/main/proto/deephaven_core/proto/remotefilesource.proto	Defines the RemoteFileSource message stream + fetch command protos.
proto/proto-backplane-grpc/Dockerfile	Includes new proto in codegen and proto-doc generation.
plugin/src/main/java/io/deephaven/plugin/type/PluginMarker.java	Introduces a generic marker object keyed by plugin name for export/type matching.
plugin/remotefilesource/src/main/java/io.deephaven.remotefilesource/RemoteFileSourceTicketResolverFactoryService.java	Registers ticket/command resolver via service loader.
plugin/remotefilesource/src/main/java/io.deephaven.remotefilesource/RemoteFileSourcePlugin.java	ObjectType plugin creating a message stream for the remote file source.
plugin/remotefilesource/src/main/java/io.deephaven.remotefilesource/RemoteFileSourceMessageStream.java	Implements bidirectional message handling and registers as a remote resource provider.
plugin/remotefilesource/src/main/java/io.deephaven.remotefilesource/RemoteFileSourceCommandResolver.java	Resolves Flight command into exported `PluginMarker` and returns FlightInfo endpoint.
plugin/remotefilesource/gradle.properties	Declares project type for the new plugin module.
plugin/remotefilesource/build.gradle	Build config + deps for the new plugin module.
go/internal/proto/remotefilesource/remotefilesource.pb.go	Generated Go protobuf bindings for new proto.
engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java	Integrates remote classloader + refreshes Groovy compilation cache when remote sources are dirty/removed.
engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java	Makes `executionContext` mutable and factors ExecutionContext construction for refresh scenarios.
engine/context/src/main/java/io/deephaven/engine/context/ExecutionContext.java	Adds `withQueryCompiler(...)` for swapping compilers while keeping context stable.
engine/base/src/main/java/io/deephaven/engine/util/RemoteFileSourceProvider.java	Defines provider interface for remote resource sourcing.
engine/base/src/main/java/io/deephaven/engine/util/RemoteFileSourceClassLoader.java	Implements resource lookup via providers using a custom `remotefile://` URL + URLConnection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        byte[] content = response.getContent().toByteArray();
+
+        log.info().append("Received resource response for requestId: ").append(requestId)
+                .append(", found: ").append(response.getFound())
+                .append(", content length: ").append(content.length).endl();
+
+        if (!response.getError().isEmpty()) {
+            log.warn().append("Error in response: ").append(response.getError()).endl();
+        }
+


+        for (String contextResourcePath : executionContext.getResourcePaths()) {
+            if (resourceName.equals(contextResourcePath)) {


+                    content = provider.requestResource(resourceName)
+                            .orTimeout(RESOURCE_TIMEOUT_SECONDS, TimeUnit.SECONDS)
+                            .get();


+            if (content == null || content.length == 0) {
+                throw new IOException("No content for resource: " + resourceName);


+
+        // Create a lazy promise that will be resolved when we get the response
+        LazyPromise<Boolean> promise = new LazyPromise<>();
+        pendingSetExecutionContextRequests.put(requestId, promise);


+import java.nio.ByteBuffer;
+


+import io.deephaven.server.session.TicketRouter;
+import io.deephaven.server.session.WantsTicketRouter;
+
+import io.grpc.StatusRuntimeException;


+
+        String requestId = UUID.randomUUID().toString();
+        CompletableFuture<byte[]> future = new CompletableFuture<>();
+        pendingRequests.put(requestId, future);


DH-20578: Adds Groovy support for remote file source features in VS Code extension. The supporting plugin work has not fully landed yet mostly waiting on tests, but I don't think that should block this PR: **Related Pending PRs** Changes for these are deployed to `bmingles-remote-file-source2` BHS - deephaven/deephaven-core#7451 - deephaven-ent/iris#3878 - TBD: PR to integrate Core changes into gplus ## Testing - Test on a grizzly or gplus server without the plugin installed. Verify that we can still run python and groovy scripts without remote file sourcing. This is just a regression test. - `bmingles-remote-file-source2` has Groovy and Python remote plugins installed, so full feature set can be tested there ### Setup - Start `bmingles-remote-file-source2` vm if not already running - Clone this test repo `git@github.com:bmingles/deephaven-controller-script-test` - Checkout the `local` git branch (the `main` branch is configured as a controller source on `bmingles-remote-file-source2`). ### Local override tests - Connect to `bmingles-remote-file-source2` server in VS Code - Open and run `src/main/groovy/docs-sample.groovy` against the BHS - Should see STDOUT entries in OUTPUT -> Deephaven panel with `"Server"` prefix e.g. ``` 12:16:14.829 STDOUT Server: Notebook Level Var 12:16:14.830 STDOUT Server: Notebook Level Var 12:16:14.830 STDOUT Server: Top Level Var 12:16:14.830 STDOUT Server: Top Level Var ... ``` - Cells in the `testTable` should also include the `"Server"` prefix - Add `src/main/groovy/test` folder as a Groovy remote file source - Run the script again. Should see `"Local"` prefix e.g. ``` 12:21:12.336 STDOUT Local: Notebook Level Var 12:21:12.336 STDOUT Local: Notebook Level Var 12:21:12.337 STDOUT Local: Top Level Var 12:21:12.337 STDOUT Local: Top Level Var ... ``` - Cells in the `testTable` should also include the `"Local"` prefix - Remove the `src/main/groovy/test` as a remote file source - Run the script again. STDOUT and table cells should all go back to the `"Server"` prefixes ### Tree Selection Behavior There was a bug fixed in this PR that impacts how partial unmarking works in the Remote file source tree view. - Navigate to Deephaven activity bar tab (left sidebar) - Expand Groovy workspace tree until you get to `src/main/groovy/nested/ws1` - Add `ws1` folder as remote source with the `+` button - Should see ws1 and all of its children turn green - Hover over `ws1/sub1` subfolder and click the `-` sign to remove it - Result should be that `ws1/sub2` is now the marked folder. `ws1/sub1` and `ws1/file1.groovy` should both be unmarked

niloc132 · 2026-06-10T16:08:08Z

There's a separate PR to this branch, bmingles#6 that adds more tests and fixes a merge bug. One of those test is failing, this PR has been blocked on not having the tests (and having them pass).

The failing use case in the test was that if a Groovy class was already present on the classpath, then the script session with the remote file source feature couldn't replace the file - the server kept using the implementation present on the classpath. The bug didn't seem to occur when running the server normally, suggesting it was a test artifact of some kind.

There appear to be two bugs in the PR itself that were preventing the test from passing, and they seem to be legit bugs that ought to prevent normal usage from working correctly as well. First, a quick call tree describing how AbstractScriptSession runs script code

AbstractScriptSession.evaluateScript(String,String) is the root for executing script code. Among other things, this takes a snapshot of the current variables, opens a query specific liveness scope, and most importantly, opens the script session's exec context.
With those done, the GroovyDeephavenSession.evaluate(String) method is called. As part of this patch, that will conditionally call refreshClassLoader(), which will erase existing classes in the class cache dir, replace the GroovyShell instance (to get a new classloader), and replace the exec context.
Then, the script code is run, within the currently open exec context

Two bugs in the above, one obvious, one less so:

The newly created exec context isn't actually opened - it is created, but not opened. I don't understand how it matters that we have a new exec context with the new query library if we won't open it until the next attempt to run groovy code?
The groovy shell is re-made, but the new classloader it came with wasn't passed to the exec context. This doesn't matter in the case where we're still using the old exec context, but if we're carrying forward the old classloader, it still won't matter.

Draft patch that resolves these two issues, and makes the test pass:

diff --git a/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java b/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
index 89f5efe59b..fbac74a81a 100644
--- a/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
+++ b/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
@@ -359,12 +359,12 @@ public class GroovyDeephavenSession extends AbstractScriptSession<GroovySnapshot
         // Create fresh classloader - reusing the existing configurations
         GroovyClassLoader scriptClassLoader = new GroovyClassLoader(STATIC_LOADER, scriptConfig);
 
-        // Update the execution context with a fresh QueryCompiler
-        final QueryCompiler freshCompiler = QueryCompilerImpl.create(classCacheDirectory, scriptClassLoader);
-        executionContext = executionContext.withQueryCompiler(freshCompiler);
-
         // Permanently replace groovyShell with fresh shell
         groovyShell = new DeephavenGroovyShell(scriptClassLoader, groovyShell.getContext(), consoleConfig);
+
+        // Update the execution context with a fresh QueryCompiler and classloader
+        final QueryCompiler freshCompiler = QueryCompilerImpl.create(classCacheDirectory, scriptClassLoader);
+        executionContext = executionContext.withQueryCompiler(freshCompiler).withClassLoader(groovyShell.getClassLoader());
     }
 
     @Override
@@ -399,8 +399,8 @@ public class GroovyDeephavenSession extends AbstractScriptSession<GroovySnapshot
         // Execute the script
         try (final SafeCloseable ignored = groovyShell.setScriptPrefix(currentScriptName)) {
             updateClassloader(lastCommand);
-            try {
-                ExecutionContext.getContext().getUpdateGraph().exclusiveLock()
+            try (SafeCloseable ignored2 = executionContext.open()){
+                executionContext.getUpdateGraph().exclusiveLock()
                         .doLockedInterruptibly(() -> groovyShell.evaluate(lastCommand));
             } catch (InterruptedException e) {
                 throw new CancellationException(e.getMessage() != null ? e.getMessage() : "Query interrupted",

More broadly though, this intersects with #8065, an attempt to fix a case where we were deleting formula class files after they had been compiled, but before they were loaded and used. This was aimed at being a bandaid fix for a customer issue, but it seems likely to find other ways to come up. Recapping the original requirement there:

Our QueryCompiler flow starts with Java-like expression strings, rebuilds them in various ways, and writes them out as Java files, and runs them through javax.tools.JavaCompiler to transform them to bytecode. JavaCompiler functionally behaves like it is running javac itself, and so must find the classpath on disk somewhere, and write the resulting class also on disk.
As we want our dynamically created groovy classes to be available from these formulas, the groovy code in turn needs to be written to disk as well (thus adding the need for this patch to manage the classes directory).

But this requirement of being able to delete classes pretty directly conflicts with our desire to keep them around - we generally should draw the line somewhere between "must keep classes around for the duration of the process or other things will break" and "don't bother writing classes to disk at all". I'm inclined to say it should be either one of the extremes, or support a third option that we haven't considered previously: "each round of classes that is written to disk goes into a new directory", enabling groovy to be rewritten at will, but formulas to stick around as needed. In theory, #8065 brings us quite close to the transient extreme, as "every formula can/should be deleted as soon as it has been compiled and loaded", which could let us decouple formula compilation output from groovy output (as formulas need to see groovy, but not each other, and groovy doesn't need to see anything). I think that could make sense as a simpler fourth option, if we think that change is rigorous enough to support lazy usage of formulas.

bmingles force-pushed the DH-20578_groovy-remote-file-sourcing branch 3 times, most recently from a48276f to fc106d6 Compare December 24, 2025 16:23

bmingles force-pushed the DH-20578_groovy-remote-file-sourcing branch from 777b0b4 to 4d1b51c Compare January 2, 2026 17:53

bmingles commented Jan 2, 2026

View reviewed changes

bmingles force-pushed the DH-20578_groovy-remote-file-sourcing branch 2 times, most recently from f0ced57 to b70f606 Compare January 5, 2026 21:15

bmingles marked this pull request as ready for review January 5, 2026 23:23

bmingles requested review from cpwright, devinrsmith, niloc132 and rcaudy as code owners January 5, 2026 23:23

bmingles requested a review from mofojed January 5, 2026 23:23

bmingles mentioned this pull request Jan 5, 2026

feat: DH-20578: Groovy remote file source deephaven/vscode-deephaven#273

Merged

niloc132 reviewed Jan 6, 2026

View reviewed changes

bmingles requested a review from niloc132 January 9, 2026 19:50

mofojed requested changes Jan 12, 2026

View reviewed changes

bmingles requested a review from mofojed January 14, 2026 00:22

mofojed reviewed Jan 14, 2026

View reviewed changes

bmingles requested a review from mofojed January 14, 2026 18:25

mofojed requested changes Jan 21, 2026

View reviewed changes

bmingles marked this pull request as draft January 22, 2026 16:18

bmingles and others added 5 commits February 23, 2026 13:40

Started building out RemoteFileSource plugin (#DH-20578)

8cca806

Command resolver now fetches plugin (#DH-20578)

b0871b7

RemoteFileSourceTicketResolverFactoryService (#DH-20578)

e183397

Added stub for JsRemoteFileSourceService (#DH-20578)

369e636

Generated GWT bindings

a093cde

bmingles added 3 commits February 25, 2026 15:48

removed unused method (#DH-20578)

a5f3af5

comments (#DH-20578)

24b0d29

spotless (#DH-20578)

168e776

bmingles marked this pull request as ready for review February 26, 2026 18:40

bmingles requested a review from mofojed February 26, 2026 19:58

mofojed requested changes Feb 27, 2026

View reviewed changes

bmingles added 3 commits March 2, 2026 18:15

Made execution context updating more targetted. (#DH-20578)

58f5919

Addressed code review comments (#DH-20578)

57e158f

Renamed RemoteFileSourceClientRequest -> RemoteFileSourceClientMessag…

38e5bae

…e and RemoteFileSourceServerRequest -> RemoteFileSourceServerMessage (#DH-20578)

bmingles requested a review from mofojed March 3, 2026 17:13

bmingles and others added 3 commits March 5, 2026 09:25

Added is_dirty flag to execution context (#DH-20578)

c522e5d

regen gwt bindings

ae41196

Merge pull request #5 from niloc132/DH-20578_groovy-remote-file-sourcing

d37be22

feat: DH-20578: regen gwt bindings

niloc132 added DocumentationNeeded groovy javascript Pull requests that update Javascript code ReleaseNotesNeeded Release notes are needed labels Mar 6, 2026

Ran ./gradlew updateProtobuf (#DH-20578)

38e7189

bmingles requested review from chipkent and jmao-denver as code owners March 11, 2026 13:44

Added isDirty flag (#DH-20578)

9664504

mofojed reviewed Mar 16, 2026

View reviewed changes

bmingles requested a review from Copilot March 17, 2026 14:57

Copilot started reviewing on behalf of bmingles March 17, 2026 14:58 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

Merge branch 'main' into DH-20578_groovy-remote-file-sourcing

9bcc8b2

		DomGlobal.setTimeout(ignore -> fireEvent(EVENT_REQUEST_SOURCE,
		new ResourceRequestEvent(message.getRequestId(), request)), 0);

		for (String contextResourcePath : executionContext.getResourcePaths()) {
		if (resourceName.equals(contextResourcePath)) {

		if (content == null \|\| content.length == 0) {
		throw new IOException("No content for resource: " + resourceName);

Conversation

bmingles commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related PRs

Testing

Community

Core+

Uh oh!

github-actions Bot commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

No docs changes detected for 9bcc8b2

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mofojed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmingles Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mofojed left a comment

Choose a reason for hiding this comment

Uh oh!

mofojed left a comment

Choose a reason for hiding this comment

Uh oh!

bmingles commented Jan 22, 2026

bmingles commented Nov 26, 2025 •

edited

Loading

github-actions Bot commented Nov 26, 2025 •

edited

Loading

No docs changes detected for `9bcc8b2`

bmingles Jan 13, 2026 •

edited

Loading

bmingles Mar 2, 2026 •

edited

Loading