-
Notifications
You must be signed in to change notification settings - Fork 20
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
COPY INTO FROM ON CLIENT #102
Conversation
The latter is no longer called by mapi.Connection.cmd().
Before we returned 0 but that leads to endless loops when called by the utf-8 codec.
They are very useful.
instead of calling codecs.getreader(). This allows us to force the line endings to "\n"
So a class can implement Uploader and Downloader simultaneously
pymonetdb/filetransfer.py
Outdated
mapi._putblock(f"Invalid file transfer command: {cmd!r}") | ||
|
||
|
||
def handle_upload(mapi, filename, text_mode, offset): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing type annotations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I've added a whole bunch of additional type annotations.
Not easy because the IO types don't type annotate well.
In particular, for some reason a BufferedIOBase
is not accepted as an IO[bytes]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting, is this with all python versions?
I just had a quick scan of the code, first impression is that It looks good, but i need to look better at the logic at some point. My first biggest comment is:
|
|
||
from io import BufferedIOBase, BufferedWriter, RawIOBase, TextIOBase, TextIOWrapper | ||
from typing import Any, Optional, Union | ||
from pymonetdb import mapi as mapi_protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just do from pymonetdb.mapi import Connection
and use that as type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gave circular import problems
This work is not finished but I'm creating the pull request so you can keep an eye on it and make suggestions, and also to benefit from the CI. The idea is to add support for ON CLIENT. For example:
With ON CLIENT, the server does not open the the file by itself but sends a request to pymonetdb to open it on its behalf. The client opens the file and reads it, or makes up some data on the spot, and sends it to the server which processes it as usual.
ON CLIENT is useful to avoid having to first copy data to a location on the server accessible both to the user and the server, and for side stepping all sorts of permissions problems. Mclient has supported it for quite a while and recently I added support to monetdb-jdbc. Now I would like to add support for it to pymonetdb.