deephaven / deephaven-core

Deephaven Community Core
Other
255 stars 81 forks source link

Better execution contexts by default #4366

Open devinrsmith opened 1 year ago

devinrsmith commented 1 year ago

The error "No ExecutionContext registered, or current ExecutionContext has no QueryLibrary" is a cryptic and user-unfriendly error. At a minimum, we should have specific documentation around how / when a python users can / should grab an existing context, or create their own execution context.

I'm also arguing that we should make it much easier for users to inherit a reasonable execution context by default without having to think about it; in this vein, we could have engine side logic that does this for groovy users too. But if we only want it for python, we could imagine instrumenting all the necessary call sites with something akin to auto_locking_ctx.

As a sketch:

    def transform(self, func: Callable[[Table], Table]) -> PartitionedTable:
        try:
            # this should be prettier after 4364
            transform_default_ctx = ExecutionContext(j_exec_ctx=_JExecutionContext.newBuilder()
                          .captureQueryCompiler()
                          .captureQueryLibrary()
                          .emptyQueryScope()
                          .setUpdateGraph(self.update_graph.j_update_graph)
                          .build())
            def transform_op(t: Table):
                with transform_default_ctx:
                    return func.call(t)
            j_operator = j_unary_operator(transform_op, dtypes.from_jtype(Table.j_object_type.jclass))
            with auto_locking_ctx(self):
                j_pt = self.j_partitioned_table.transform(j_operator)
                return PartitionedTable(j_partitioned_table=j_pt)
        except Exception as e:
            raise DHError(e, "failed to transform the PartitionedTable.") from e

This sketch still allows the user to install their own execution context in func if they desire.

Related to / depends on https://github.com/deephaven/deephaven-core/issues/4364

rcaudy commented 1 year ago

I'm unsure what the scope of "by default" is. We don't know when users make their own threads...