Skip to content
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

rename table client -> engine interface #107

Merged
merged 4 commits into from
Jan 31, 2024

Conversation

nicklan
Copy link
Collaborator

@nicklan nicklan commented Jan 24, 2024

Simple rename of TableClient to EngineInterface and table_client to engine_interface.

I was going to do this as part of the big PR for data passing, but I think actually doing this first will make the review less painful, so I've pulled it out.

Doesn't touch DefaultTableClient yet, that can be renamed separately.

Copy link
Collaborator

@roeap roeap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@ryan-johnson-databricks ryan-johnson-databricks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely an improvement, but part of me wonders whether "client" is just plain the wrong word? This is more like an EngineInterface concept, no?

Copy link
Collaborator

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to throw another idea out: what about just Engine trait?

@roeap
Copy link
Collaborator

roeap commented Jan 30, 2024

people may be tired of hearing this, but there happens to be a somewhat famous saying around naming things in CS 😆.

Just thinking out loud here, but to me, Client immediately invokes thinking about client-server interactions, which may not be what we are doing here.

I guess what we are asking from the engine is to provide us with a set of capabilities - implement a specific API - which may a case for EngineInterface.

Since we are essentially saying, that the engine side of things should do all the heavy lifting - i.e. without it out actions are just empty statements, it may be called just Engine as well.

Somehow the thing that is somehow special to me here, that we are also asking the engine to provide us with a data format it wants to work with, so its not only implementing the execution of actions but also what these actions should act on ...

not sure if this is correct, but Kernel invokes the notion of an OS, no? I may be wrong here, but if an OS interacts with another system would that be what is called a Driver?

Looking at may ramblings this may not even be helpful 😆. I guess EngineInterface would maybe be the most generic term, which would let us call the existing system the engine, which implements the engine interface ...

@roeap
Copy link
Collaborator

roeap commented Jan 30, 2024

one more thought came up - in query engine terms, is kernel maybe doing the planning, and the engine is doing the execution?

@roeap
Copy link
Collaborator

roeap commented Jan 30, 2024

going out on a limb here, and thinking also about the ffi case. since the top level thing is not doing anything and just providing us access to the specific sub capabilities - is it maybe just an EngineHandle?

@ryan-johnson-databricks
Copy link
Contributor

going out on a limb here, and thinking also about the ffi case. since the top level thing is not doing anything and just providing us access to the specific sub capabilities - is it maybe just an EngineHandle?

It's quite likely FFI code will surface a notion of "handle" soon, to identify data structures defined by kernel or engine that are opaque to the other. Hopefully without giving up the last remnants of strong typing. That's still under heavy prototyping tho, so we probably shouldn't wait for it to stabilize before merging this PR.

@nicklan nicklan force-pushed the table-to-engine-client branch from 6c9efe1 to c58d1e4 Compare January 31, 2024 00:18
Copy link
Collaborator

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nicklan nicklan changed the title rename table client -> engine client rename table client -> engine interface Jan 31, 2024
Copy link
Contributor

@ryan-johnson-databricks ryan-johnson-databricks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

acceptance/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Ryan Johnson <[email protected]>
@nicklan nicklan merged commit d8d3eb3 into delta-io:main Jan 31, 2024
3 checks passed
@nicklan nicklan deleted the table-to-engine-client branch January 31, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants