-
Notifications
You must be signed in to change notification settings - Fork 4
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
Models action from v1 to v2 #101
base: main
Are you sure you want to change the base?
Changes from 13 commits
1967d0d
ead5b03
806fde3
e048fd6
e081c04
a87fe47
cb85a1a
5abc0f9
35b4997
a6cf17d
747a058
e9d254f
19a397c
18b9c76
6e25efb
fcaf17c
9289342
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,9 +19,13 @@ | |
import time | ||
import re | ||
import io | ||
import sys | ||
import random | ||
|
||
from enum import Enum, unique | ||
from typing import List, Union | ||
from requests_toolbelt import multipart | ||
|
||
from . import rest | ||
|
||
from .pb.message_pb2 import MetadataInfo | ||
|
@@ -114,7 +118,7 @@ class Permission(str, Enum): | |
"create_oauth_client", | ||
"delete_database", | ||
"delete_engine", | ||
"delete_model", | ||
"delete_models", | ||
"disable_user", | ||
"enable_user", | ||
"delete_oauth_client", | ||
|
@@ -598,18 +602,6 @@ def run(self, ctx: Context, command: str, language: str, inputs: dict = None) -> | |
raise Exception("invalid response type") | ||
|
||
|
||
def _delete_model_action(name: str) -> dict: | ||
return {"type": "ModifyWorkspaceAction", "delete_source": [name]} | ||
|
||
|
||
def _install_model_action(name: str, model: str) -> dict: | ||
return {"type": "InstallAction", "sources": [_model(name, model)]} | ||
|
||
|
||
def _list_action(): | ||
return {"type": "ListSourceAction"} | ||
|
||
|
||
def _list_edb_action(): | ||
return {"type": "ListEdbAction"} | ||
|
||
|
@@ -657,43 +649,83 @@ def _model(name: str, model: str) -> dict: | |
} | ||
|
||
|
||
# Returns full list of models. | ||
def _list_models(ctx: Context, database: str, engine: str) -> dict: | ||
tx = Transaction(database, engine, mode=Mode.OPEN) | ||
rsp = tx.run(ctx, _list_action()) | ||
actions = rsp["actions"] | ||
assert len(actions) == 1 | ||
action = actions[0] | ||
models = action["result"]["sources"] | ||
return models | ||
|
||
|
||
def create_database(ctx: Context, database: str, source: str = None) -> dict: | ||
data = {"name": database, "source_name": source} | ||
url = _mkurl(ctx, PATH_DATABASE) | ||
rsp = rest.put(ctx, url, data) | ||
return json.loads(rsp.read()) | ||
|
||
|
||
def delete_model(ctx: Context, database: str, engine: str, model: str) -> dict: | ||
tx = Transaction(database, engine, mode=Mode.OPEN, readonly=False) | ||
actions = [_delete_model_action(model)] | ||
return tx.run(ctx, *actions) | ||
# Returns full list of models. | ||
def list_models(ctx: Context, database: str, engine: str) -> List: | ||
models = [] | ||
out_name = f'model{random.randint(0, sys.maxsize)}' | ||
resp = exec(ctx, database, engine, f'def output:{out_name}[name] = rel:catalog:model(name, _)') | ||
for result in resp.results: | ||
if f'/:output/:{out_name}' in result['relationId']: | ||
table = result['table'].to_pydict() | ||
models.extend([table['v1'][i] for i in range(1, len(table['v1']))]) | ||
|
||
return models | ||
|
||
|
||
def delete_models(ctx: Context, database: str, engine: str, models: List[str]) -> TransactionAsyncResponse: | ||
queries = [ | ||
f'def delete:rel:catalog:model["{model_name}"] = rel:catalog:model["{model_name}"]' | ||
for model_name in models | ||
] | ||
return exec(ctx, database, engine, '\n'.join(queries), readonly=False) | ||
|
||
|
||
def delete_models_async(ctx: Context, database: str, engine: str, models: List[str]) -> TransactionAsyncResponse: | ||
queries = [ | ||
f'def delete:rel:catalog:model["{model_name}"] = rel:catalog:model["{model_name}"]' | ||
for model_name in models | ||
] | ||
return exec_async(ctx, database, engine, '\n'.join(queries), readonly=False) | ||
|
||
|
||
# Returns the named model | ||
def get_model(ctx: Context, database: str, engine: str, name: str) -> str: | ||
models = _list_models(ctx, database, engine) | ||
for model in models: | ||
if model["name"] == name: | ||
return model["value"] | ||
out_name = f'model{random.randint(0, sys.maxsize)}' | ||
cmd = f'def output:{out_name} = rel:catalog:model["{name}"]' | ||
resp = exec(ctx, database, engine, cmd) | ||
for result in resp.results: | ||
if f'/:output/:{out_name}' in result['relationId']: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's the reason why we generated a random output name then we filter out results based on that |
||
table = result['table'].to_pydict() | ||
return table['v1'][0] | ||
raise Exception(f"model '{name}' not found") | ||
|
||
|
||
def install_model(ctx: Context, database: str, engine: str, models: dict) -> dict: | ||
tx = Transaction(database, engine, mode=Mode.OPEN, readonly=False) | ||
actions = [_install_model_action(name, model) for name, model in models.items()] | ||
return tx.run(ctx, *actions) | ||
def install_models(ctx: Context, database: str, engine: str, models: dict) -> TransactionAsyncResponse: | ||
queries = [] | ||
queries_inputs = {} | ||
randint = random.randint(0, sys.maxsize) | ||
index = 0 | ||
for name, value in models.items(): | ||
input_name = f'input_{randint}_{index}' | ||
queries.append(f'def delete:rel:catalog:model["{name}"] = rel:catalog:model["{name}"]') | ||
queries.append(f'def insert:rel:catalog:model["{name}"] = {input_name}') | ||
|
||
queries_inputs[input_name] = value | ||
index += 1 | ||
|
||
return exec(ctx, database, engine, '\n'.join(queries), inputs=queries_inputs, readonly=False) | ||
|
||
|
||
def install_models_async(ctx: Context, database: str, engine: str, models: dict) -> TransactionAsyncResponse: | ||
queries = [] | ||
queries_inputs = {} | ||
randint = random.randint(0, sys.maxsize) | ||
index = 0 | ||
for name, value in models.items(): | ||
input_name = f'input_{randint}_{index}' | ||
queries.append(f'def delete:rel:catalog:model["{name}"] = rel:catalog:model["{name}"]') | ||
queries.append(f'def insert:rel:catalog:model["{name}"] = {input_name}') | ||
|
||
queries_inputs[input_name] = value | ||
index += 1 | ||
return exec_async(ctx, database, engine, '\n'.join(queries), inputs=queries_inputs, readonly=False) | ||
|
||
|
||
def list_edbs(ctx: Context, database: str, engine: str) -> list: | ||
|
@@ -706,12 +738,6 @@ def list_edbs(ctx: Context, database: str, engine: str) -> list: | |
return rels | ||
|
||
|
||
# Returns a list of models installed in the given database. | ||
def list_models(ctx: Context, database: str, engine: str) -> list: | ||
models = _list_models(ctx, database, engine) | ||
return [model["name"] for model in models] | ||
|
||
|
||
# Generate a rel literal relation for the given dict. | ||
def _gen_literal_dict(items: dict) -> str: | ||
result = [] | ||
|
@@ -879,6 +905,6 @@ def exec_async( | |
get_compute = get_engine # deprecated, use get_engine | ||
list_computes = list_engines # deprecated, use list_engines | ||
list_edb = list_edbs # deprecated, use list_edbs | ||
delete_source = delete_model # deprecated, use delete_model | ||
delete_source = delete_models # deprecated, use delete_model | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this changes the signature (i believe) .. so using this trick to support old names doesnt work, as its still breaking. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh totally missed that, yep unfortunately this a breaking change, we have two options here:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. finally added an equivalent |
||
get_source = get_model # deprecated, use get_model | ||
list_sources = list_models # deprecated, use list_models |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
ed25519==1.5 | ||
grpcio-tools==1.47.0 | ||
protobuf==3.20.1 | ||
protobuf==3.20.2 | ||
pyarrow==6.0.1 | ||
requests-toolbelt==0.9.1 | ||
|
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 does this qualify the output relation?
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.
we qualify the output relation here to avoid collision with user predefined outputs
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.
👍 The JS sdk does something similar, that makes sense to me.