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

Update installModels to go through the v2 API endpoints #38

Open
NHDaly opened this issue Aug 3, 2022 · 2 comments
Open

Update installModels to go through the v2 API endpoints #38

NHDaly opened this issue Aug 3, 2022 · 2 comments

Comments

@NHDaly
Copy link
Member

NHDaly commented Aug 3, 2022

async installModels(database: string, engine: string, models: Model[]) {
const action: InstallAction = {
type: 'InstallAction',
sources: models,
};
return await this.runActions(database, engine, [action], false);
}
async listModels(database: string, engine: string) {
const action: ListSourceAction = {
type: 'ListSourceAction',
};
return await this.runActions(database, engine, [action]);
}

It's currently using the v1 actions API. It should instead be issuing an exec() query that loads the models, similar to this implementation of load_json in the julia SDK:
https://github.com/RelationalAI/rai-sdk-julia/blob/6c9ddf55fe791d82b15c8a9e9f80c0bcdec04064/src/api.jl#L650-L655

Ideally it should construct a query string + inputs, that would look something like either of these options:

query = `def insert:rel:catalog:model = model_insertions`;
inputs = {
    name: "model_insertions",
    value: models.map((m, index) => {
        [m.name, m.value],
    })
};

or like this (copied from the current Console code):

  models.forEach((m, index) => {
    const inputRelation = `__model_value__${index}__`;

    queryStrings.push(
      `def delete:rel:catalog:model["${m.name}"] = rel:catalog:model["${m.name}"]`,
      `def insert:rel:catalog:model["${m.name}"] = ${inputRelation}`,
    );
    queryInputs.push({
      name: inputRelation,
      value: m.value,
    });
  });

https://github.com/RelationalAI/rai-ux/blob/2ed5627959f793b19f5f968d8113509c8610e548/console/src/utils/sdkUtils.ts#L310-L330

@NHDaly NHDaly added type:feature New feature or request priority:medium labels Aug 3, 2022
@NHDaly NHDaly assigned denisgursky and NRHelmi and unassigned denisgursky and NRHelmi Aug 10, 2022
@NHDaly
Copy link
Member Author

NHDaly commented Aug 10, 2022

@denisgursky I've assigned this one to you, and put it at medium priority. It would be great to have this converted so that we can see a dent in our v1 vs v2 numbers 😭

@billscheidel-rai
Copy link

Note: This issue has been migrated to https://relationalai.atlassian.net/browse/RAI-6246.

This link is only accessible to employees of RelationalAI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants