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

Batch version of the get endpoint #239

Open
mastercoms opened this issue Jul 15, 2023 · 6 comments
Open

Batch version of the get endpoint #239

mastercoms opened this issue Jul 15, 2023 · 6 comments

Comments

@mastercoms
Copy link

We are facing performance issues with needing to do multiple requests to get different query parameters. For example requesting multiple foreign key matches.

It would be a big help to us if we could provide a list of JSON query objects and receive back a list of results, mapped in order to the query objects. So essentially running each query object as a separate query and listing out each rows response.

@dantownsend
Copy link
Member

Was the problem that if we need to specify too many values for __visible_fields we exceed the max URL size?

GET /movie/?__visible_fields=id,name,director.name

I know we discussed adding an all option:

GET /movie/?__visible_fields=id,name,director.all

Is that enough to solve the problem? I thought about allowing the __visible_fields to be specified as a header as well - not sure if that would help.

In terms of allowing multiple queries in one go, we could add something like PiccoloCRUDAggregate.

Some rough pseudo-code:

class PiccoloCRUDAggregate:
    def __init__(self, crud_classes: list[PiccoloCRUD]):
        self.crud_classes = crud_classes

    def post(self, payload):
        response = []
        for query in payload.queries:
            response.append(self.some_crud_class.process_query(query))
       return response

We would call it like:

POST /crud-aggregate/
{
    "queries": [
         "/movies/?__visible_fields=name,director.name",
         "/actors/?__visible_fields=name",
    ]
}

That would allow multiple requests to be processed in one go (almost GraphQL like), but I'm not sure if that's what you had in mind.

@mastercoms
Copy link
Author

This is separate from the earlier visible fields feedback, we just are needing rows back from multiple different matches. The aggregate function would work but I'd rather one for each model for better type support.

Query would be like

POST /movies/batch/get

{
queries: [
 {
    director: "Bay",
    director__match: "exact"
  },
  {
    director: "Abrams",
    director__match: "exact"
  }
]
}

Pseudo code:

def post(self, payload):
  response = []
  for query in payload.queries;
    response.append(self.base_method(convert_dict_to_query_params(query)))
  return response 
  

@dantownsend
Copy link
Member

Yeah, I like that.

I'm just thinking about what the endpoint's URL should be.

POST /movies/batch/get
POST /movies/query/
POST /movies/search/

@mastercoms
Copy link
Author

mastercoms commented Jul 15, 2023

It could also be /movies/batch then add a method: "GET" in the body. Though that might make types confusing.

I think since there is already bulk delete and an upcoming bulk update, it would probably make more sense to make a unique name like you've suggested in those last two examples. I like query more than search, since search implies something else in my mind.

@dantownsend
Copy link
Member

I had a quick play around with this.

My main challenge now is creating a Pydantic model (which is used in the OpenAPI schema if using FastAPIWrapper) for this:

{
  "queries": [
    {
      "director": "Bay",
      "director__match": "exact"
    },
    {
      "director": "Abrams",
      "director__match": "exact"
    }
  ]
}

This would be easier:

{
    "queries": [
         "director=Bay&director__match=exact",
         "director=Abrams&director__match=exact"
    ]
}

So the model is just:

class QueryModel(BaseModel):
    queries: list[str]

But I think having it an object is preferable. Will play around some more.

@mastercoms
Copy link
Author

That's already amazing progress! Thanks so much!

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

No branches or pull requests

2 participants