Move models actions to v2 protocol #35
Conversation
rai/client.go
Outdated
| tx := NewTransaction(c.Region, database, engine, "OPEN") | ||
| data := tx.Payload(makeListModelsAction()) | ||
| err := c.Post(PathTransaction, tx.QueryArgs(), data, &result) | ||
| models, err := c.ListModels(database, engine) |
There was a problem hiding this comment.
Why would we query out all models just to return a single one? We should issue a separate query here targeting a single model. This is no different than if a database has 1,000,000 products in it. You wouldn't query out all 1,000,000 just to return a single one.
There was a problem hiding this comment.
that's right 😬 will fix this
There was a problem hiding this comment.
this was done previously as it was the only way to retrieve a single model, with the expectation that we would eventually replace with an appropriate API call.
rai/client.go
Outdated
| tx := NewTransaction(c.Region, database, engine, "OPEN") | ||
| data := tx.Payload(makeListModelsAction()) | ||
| err := c.Post(PathTransaction, tx.QueryArgs(), data, &models) | ||
| modelNames := make([]string, 0) |
There was a problem hiding this comment.
Similar issue here. We shouldn't query out the models and all their source code just to return the names. We can write a query to only return the names.
rai/client.go
Outdated
| result = append(result, model.Name) | ||
|
|
||
| for _, res := range resp.Results { | ||
| if strings.Contains(res.RelationID, "/:output/:__models__") { |
There was a problem hiding this comment.
Why are we using weird relation names like __models__?
There was a problem hiding this comment.
will update this to /:output/:models
rai/client.go
Outdated
| tx := NewTransaction(c.Region, database, engine, "OPEN") | ||
| data := tx.Payload(makeListModelsAction()) | ||
| err := c.Post(PathTransaction, tx.QueryArgs(), data, &result) | ||
| models, err := c.ListModels(database, engine) |
There was a problem hiding this comment.
this was done previously as it was the only way to retrieve a single model, with the expectation that we would eventually replace with an appropriate API call.
rai/client.go
Outdated
| data := tx.Payload(makeListModelsAction()) | ||
| err := c.Post(PathTransaction, tx.QueryArgs(), data, &models) | ||
| models := make([]Model, 0) | ||
| resp, err := c.Execute(database, engine, "def output:__models__ = rel:catalog:model", nil, true) |
There was a problem hiding this comment.
again, why are we qualifying the output relation here?
There was a problem hiding this comment.
oh this is an old commit, we no longer query for all models (including name and source)
There was a problem hiding this comment.
we qualify output relation to prevent accidental collisions between output results
Use v2 protocol for model actions