Models action from v1 to v2 #101
Conversation
| 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, _)') |
There was a problem hiding this comment.
why does this qualify the output relation?
There was a problem hiding this comment.
we qualify the output relation here to avoid collision with user predefined outputs
There was a problem hiding this comment.
👍 The JS sdk does something similar, that makes sense to me.
| 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.
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
railib/api.py
Outdated
| 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.
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.
oh totally missed that, yep unfortunately this a breaking change, we have two options here:
- keep supporting
delete_sourceby adding an equivalentdelete_model - deprecate
delete_sourceand 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 ?
There was a problem hiding this comment.
finally added an equivalent delete_model
3 participants
Use v2 protocol for model actions