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

feat: Mosaic server in Python #287

Merged
merged 25 commits into from
Feb 9, 2024
Merged

feat: Mosaic server in Python #287

merged 25 commits into from
Feb 9, 2024

Conversation

domoritz
Copy link
Member

@domoritz domoritz commented Feb 2, 2024

  • working websocket
  • create and load budles
  • basic unit tests
  • http
  • cache to disk

@jheer
Copy link
Member

jheer commented Feb 2, 2024

This is looking great. Thank you!!

Would it help to name the package folder “python-server” or “python-duckdb-server” to better indicate what the package provides?

@domoritz
Copy link
Member Author

domoritz commented Feb 3, 2024

Totally unscientific but the Python server seems to perform pretty well. Here are the logs of me brushing back and forth a bit. I tried a version of the python server with https://websockets.readthedocs.io/ and it wasn't as fast as the one with socketify but again I didn't do any clean experiments.

Node server

Screenshot 2024-02-03 at 10 36 06

Python server

Screenshot 2024-02-03 at 10 36 40

@domoritz
Copy link
Member Author

domoritz commented Feb 3, 2024

Would it help to name the package folder “python-server” or “python-duckdb-server” to better indicate what the package provides?

I prefer not to have the programming language in the package name. But we could name it duckdb-server. However, we may extend what the server can do at some point so it might make sense to keep it generic.

I was also thinking we would replace the existing server completely. Is there a strong reason to keep the node sever?

@jheer
Copy link
Member

jheer commented Feb 4, 2024

I prefer not to have the programming language in the package name. But we could name it duckdb-server. However, we may extend what the server can do at some point so it might make sense to keep it generic.

I'd rather error on the side of being helpfully specific, so maybe duckdb-server?

I was also thinking we would replace the existing server completely. Is there a strong reason to keep the node server?

I definitely want to keep the DuckDB node client class in the mosaic-duckdb package. I'm already using it in other experiments (for example, to instantiate a running Coordinator in Node that talks directly to a DuckDB node instance). It's less clear to me what we want to do with the Node server in the long term. I think we should keep it for now and reassess once we see (1) experiences with a Python-based server, and (2) what fixes/issues we see with DuckDB+Arrow+Node in DuckDB v10+.

@domoritz domoritz marked this pull request as ready for review February 8, 2024 01:12
@domoritz
Copy link
Member Author

domoritz commented Feb 8, 2024

@jheer The server should be feature complete now. There are a bunch of more things we can do but I think this is a good starting point for more experiments.

@domoritz domoritz requested a review from jheer February 8, 2024 19:52
@jheer jheer merged commit b3907fd into main Feb 9, 2024
2 checks passed
@jheer jheer deleted the dom/python-server branch February 9, 2024 03:46
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.

2 participants