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

Models action from v1 to v2 #101

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
5 changes: 2 additions & 3 deletions examples/delete_model.py → examples/delete_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,15 @@
"""Delete the given Rel model from the given database."""

from argparse import ArgumentParser
import json
from urllib.request import HTTPError
from railib import api, config, show


def run(database: str, engine: str, model: str, profile: str):
cfg = config.read(profile=profile)
ctx = api.Context(**cfg)
rsp = api.delete_model(ctx, database, engine, model)
print(json.dumps(rsp, indent=2))
rsp = api.delete_models(ctx, database, engine, [model])
print(rsp)


if __name__ == "__main__":
Expand Down
5 changes: 2 additions & 3 deletions examples/install_model.py → examples/install_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
"""Install the given Rel model in the given database"""

from argparse import ArgumentParser
import json
from os import path
from urllib.request import HTTPError
from railib import api, config, show
Expand All @@ -32,8 +31,8 @@ def run(database: str, engine: str, fname: str, profile: str):
models[_sansext(fname)] = fp.read() # model name => model
cfg = config.read(profile=profile)
ctx = api.Context(**cfg)
rsp = api.install_model(ctx, database, engine, models)
print(json.dumps(rsp, indent=2))
rsp = api.install_models(ctx, database, engine, models)
print(rsp)


if __name__ == "__main__":
Expand Down
4 changes: 2 additions & 2 deletions examples/run-all
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ python3 ./show_results.py $DATABASE $ENGINE
python3 ./show_problems.py $DATABASE $ENGINE

# load model
python3 ./install_model.py $DATABASE $ENGINE hello.rel
python3 ./install_models.py $DATABASE $ENGINE hello.rel
python3 ./get_model.py $DATABASE $ENGINE hello
python3 ./list_models.py $DATABASE $ENGINE
python3 ./delete_model.py $DATABASE $ENGINE hello
Expand All @@ -60,7 +60,7 @@ python3 ./list_edbs.py $DATABASE $ENGINE
python3 ./delete_database.py $DATABASE
python3 ./create_database.py $DATABASE
python3 ./load_json.py $DATABASE $ENGINE sample.json -r sample_json
python3 ./install_model.py $DATABASE $ENGINE hello.rel
python3 ./install_models.py $DATABASE $ENGINE hello.rel
python3 ./clone_database.py $DATABASE_CLONE $DATABASE
python3 ./get_database.py $DATABASE_CLONE
python3 ./list_databases.py
Expand Down
112 changes: 69 additions & 43 deletions railib/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"}

Expand Down Expand Up @@ -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, _)')
Copy link
Collaborator

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?

Copy link
Contributor Author

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

Copy link
Member

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.

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']:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
users can install predefined output relations that could collapse with the get_model one

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:
Expand All @@ -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 = []
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:

  • keep supporting delete_source by adding an equivalent delete_model
  • deprecate delete_source and remove it from the api as it is also removed from other SDKs
    I think the first option makes more sense, @bradlo what do you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finally added an equivalent delete_model

get_source = get_model # deprecated, use get_model
list_sources = list_models # deprecated, use list_models
2 changes: 1 addition & 1 deletion requirements.txt
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

2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"ed25519==1.5",
"pyarrow>=6.0.1",
"requests-toolbelt==0.9.1",
"protobuf==3.20.1"],
"protobuf==3.20.2"],
license="http://www.apache.org/licenses/LICENSE-2.0",
long_description="Enables access to the RelationalAI REST APIs from Python",
long_description_content_type="text/markdown",
Expand Down
48 changes: 34 additions & 14 deletions tests/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,18 @@ def create_engine_wait(ctx: api.Context, engine: str):

ctx = api.Context(**cfg)

suffix = uuid.uuid4()
engine = f"python-sdk-{suffix}"
dbname = f"python-sdk-{suffix}"


class TestTransactionAsync(unittest.TestCase):
def setUp(self):
create_engine_wait(ctx, engine)
api.create_database(ctx, dbname)
self.suffix = uuid.uuid4()
self.engine = f"python-sdk-{self.suffix}"
self.dbname = f"python-sdk-{self.suffix}"
create_engine_wait(ctx, self.engine)
api.create_database(ctx, self.dbname)

def test_v2_exec(self):
cmd = "x, x^2, x^3, x^4 from x in {1; 2; 3; 4; 5}"
rsp = api.exec(ctx, "hnr-db", "hnr-engine", cmd)
rsp = api.exec(ctx, self.dbname, self.engine, cmd)

# transaction
self.assertEqual("COMPLETED", rsp.transaction["state"])
Expand All @@ -80,15 +79,36 @@ def test_v2_exec(self):
# results
self.assertEqual(
{
'v1': [
1, 2, 3, 4, 5], 'v2': [
1, 4, 9, 16, 25], 'v3': [
1, 8, 27, 64, 125], 'v4': [
1, 16, 81, 256, 625]}, rsp.results[0]["table"].to_pydict())
'v1': [1, 2, 3, 4, 5],
'v2': [1, 4, 9, 16, 25],
'v3': [1, 8, 27, 64, 125],
'v4': [1, 16, 81, 256, 625]
},
rsp.results[0]["table"].to_pydict())

def test_models(self):
models = api.list_models(ctx, self.dbname, self.engine)
self.assertTrue(len(models) > 0)

models = {'test_model': 'def foo=:bar'}
resp = api.install_models(ctx, self.dbname, self.engine, models)
self.assertEqual(resp.transaction['state'], 'COMPLETED')

value = api.get_model(ctx, self.dbname, self.engine, 'test_model')
self.assertEqual(models['test_model'], value)

models = api.list_models(ctx, self.dbname, self.engine)
self.assertTrue('test_model' in models)

resp = api.delete_models(ctx, self.dbname, self.engine, ['test_model'])
self.assertEqual(resp.transaction['state'], 'COMPLETED')

models = api.list_models(ctx, self.dbname, self.engine)
self.assertFalse('test_model' in models)

def tearDown(self):
api.delete_engine(ctx, engine)
api.delete_database(ctx, dbname)
api.delete_engine(ctx, self.engine)
api.delete_database(ctx, self.dbname)


if __name__ == '__main__':
Expand Down