Miksus / red-bird

Repository Patterns for Python
https://red-bird.readthedocs.io
MIT License
163 stars 22 forks source link

Field `id_field` is not optional. Missing pagination? #30

Open ClimenteA opened 2 years ago

ClimenteA commented 2 years ago

Awesome work!

I tried redbird with mongorepo and noticed that the id_field is not optional. Is there any reason behind that?

The id_field is set to _id field on mongo which turns of some features of mongo (mongo generates faster the _id and makes it easier to sort).

I assume that this is because ObjectId from mongo is not json parsable?

Also, I didn't found any options to use limit and skip together for pagination (there is just limit).

Miksus commented 2 years ago

Thanks! It's always nice to hear people like it!

I remember the _id was actually quite problematic one with Pydantic (in MongoRepo). I think I had quite a lot problems with Pydantic thinking it's private due to the underscore and also that the ObjectId does not align well with default Pydantic data conversions.

I cannot remember the exact point of it (id_field) being not optional, I can take a look in the weekend, but at this moment I don't think it necessarily should be required.

And what comes to skip, I think that could also be implemented but it should be done in a way that feels "native": maybe a keyword with limit/all as possibly another method pipelining could get too nested and is a lot to implement. Even though the underlying query mechanism in other repos might not support it (like some SQL dialects possibly) they could just implement it by a simple Python loop. Possibly inefficient but the purpose of this package is not to be top-notch optimized but simple to use and works the same regardless of the data store.

Thanks a lot for the suggestions! Those were good points you brought up.

ClimenteA commented 2 years ago

Here is a quick hack I come up to handle the mongo ObjectId. Maybe this can help a bit.

import json
from bson.json_util import dumps
from bson.objectid import ObjectId

def valid_object_id(oid_string):
    try:
        ObjectId(oid_string)
        return True
    except:
        return False

def parse_oid(oid):
    if isinstance(oid, ObjectId):
        return json.loads(dumps(oid))['$oid']
    return oid