-
Notifications
You must be signed in to change notification settings - Fork 8
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
[Python] Add command-line program to work with the CrateDB BLOB store #86
base: main
Are you sure you want to change the base?
Conversation
actions = parser.add_subparsers( | ||
dest="action", | ||
title="action", | ||
description="valid subcommands", | ||
help="additional help", | ||
) |
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.
Nit: Define title
, description
, and help
better.
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.
do it? ;)
""" | ||
What the function name says. | ||
""" |
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.
Meaningless function doc?
""" | |
What the function name says. | |
""" |
|
||
def truncate(value: bytes) -> bytes: | ||
""" | ||
Truncate long string. |
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.
Truncate long string. | |
Truncate long string to be used for logging purpose. |
|
||
# Define arbitrary content for testing purposes. | ||
content = "An example payload.".encode("utf-8") | ||
# content = b"\xde\xad\xbe\xef" * 256 |
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.
remove?
actions = parser.add_subparsers( | ||
dest="action", | ||
title="action", | ||
description="valid subcommands", | ||
help="additional help", | ||
) |
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.
do it? ;)
def refresh(self): | ||
""" | ||
Optionally synchronize data after write operations. | ||
""" | ||
self.run_sql(f'REFRESH TABLE "{self.name}";') |
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.
looks unused, remove it?
Otherwise I think it would have to use the blob
schema, such as REFRESH TABLE blob."{self.name}";
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.
Yeah, I also recognized this detail yesterday. Thank you. So, effectively, there is no way to run a BLOB table in a different schema? It is always blob
?
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.
Yes it's always blob
, see also https://cratedb.com/docs/crate/reference/en/5.4/sql/statements/create-blob-table.html (syntax does not support to define a schema name).
FYI: I've just reused that basic implementation over at crate/mlflow-cratedb#33, adding more features to implement an object store. |
About
The guideline at the Python client documentation about working with the CrateDB BLOB store is sweet, but a runnable program is even better.
Synopsis
Backlog
we may want to think about shipping it in one way or another,
because it is a good building block.
/cc @andnig, @hlcianfagna, @hammerhead, @karynzv, @marijaselakovic, @proddata