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

Backend public API #2678

Closed
datapythonista opened this issue Mar 10, 2021 · 2 comments · Fixed by #3038
Closed

Backend public API #2678

datapythonista opened this issue Mar 10, 2021 · 2 comments · Fixed by #3038
Labels
backends Issues related to all backends refactor Issues or PRs related to refactoring the codebase
Milestone

Comments

@datapythonista
Copy link
Contributor

At the moment there is no clear public API for the backends. Backends so far implement a connect method that returns a Client class. That's not required, since what is exposed at the moment is the backend module itself, so it's possible to implement a backend like:

ibis.snowflake.start(server='whatever')

Which can return something totally inconsistent with the rest of Client classes of the backends. While so far backend developers tried to be consistent, and there are inconsistencies, but small, and probably created by mistake, I think we can do better.

In a first effort, I created a class named Backend to be exposed instead of the models, and all backends subclass an ABC, so some methods are guaranteed to exist, like the connect method. While IMO this is a good first step, and helped standardize things, like the compile and verify methods, which are now part of the ABC, ensuring consistency, and avoiding a decent amount of duplicate code.

I've been checking, and what it would make sense to me is to merge this Backend class (which used to be the functions in the backend module) with the Client classes. They became separate for historical reasons on the API being exposed as the backend, but I think the new resulting class should be what we define as the public backend API.

I'm unsure of what's the best name for the new class, I think good candidates are:

  • Backend
  • Client
  • Connection

Given code like this:

connection = ibis.connect('sqlite://db.sqlite3')

Feels like the most natural name for the class is connection, instead of the current Client or Backend. But all seem reasonable.

And I think it should implement the functionality to do things like:

  • connect
  • list_databases
  • database
  • list_tables
  • table
  • compile

And probably other optional things like insert, drop_table...

If that sounds good with everyone, and there are no strong preferences about the name, I'll be implementing this, and call the new class Connection.

@datapythonista datapythonista added refactor Issues or PRs related to refactoring the codebase backends Issues related to all backends labels Mar 10, 2021
@datapythonista datapythonista added this to the Next release milestone Mar 10, 2021
@tswast
Copy link
Collaborator

tswast commented May 5, 2021

Would "connect" return this class? That is, would it be a classmethod factory method?

Or is there still a Client class that is returned by it?

@datapythonista
Copy link
Contributor Author

Open to discussion, but personally I think connect should be a function, and return a class (a Connection class, which would be the union of the current Backend and Client). Happy to hear other opinions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backends Issues related to all backends refactor Issues or PRs related to refactoring the codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants