Why do we use `jsonable_encoder` in crud/base:update

[haykkh]

I'm working through a project that was (at one point a long time ago) based on this stack.

https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/490c554e23343eec0736b06e59b2108fdd057fdc/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/crud/base.py#L49

I've run into an issue with SQLAlchemy hybrid_properties not being included when jsonable_encoder is called on db_obj. This got me thinking as to why we even encode db_obj into obj_data here, seeing as we only use obj_data to iterate through the attributes of db_obj.

To fix this issue I've removed the jsonable_encoder logic here, and instead check if db_obj contains the attribute with hasattr (see below). Am I missing something obvious as to why I should be using jsonable_encoder here? Or are my modifications safe?

My changes:

def update(
    self,
    db: Session,
    *,
    db_obj: ModelType,
    obj_in: Union[UpdateSchemaType, Dict[str, Any]]
) -> ModelType:
    if isinstance(obj_in, dict):
        update_data = obj_in
    else:
        update_data = obj_in.dict(exclude_unset=True)
    for field in update_data:
        if hasattr(db_obj, field):
            setattr(db_obj, field, update_data[field])
    db.add(db_obj)
    db.commit()
    db.refresh(db_obj)
    return db_obj

[shulcsm]

Makes sense. Also, i think missing attr should result in exception.

[krconv]

I am curious of the same thing; ran into two separate issues caused by this;

• We use factory_boy for unit testing, and it seems like the sqlalchemy models produced by it lazy-load the fields; so, jsonable_encoder ends up not including them and then the update doesn't actually do anything (because for field in obj_data didn't include it)
• We have a model with a circular reference (user refers to auth, auth to user), and if the field was accessed (i.e. user.auth) then jsonable_encoder will hit a recursion error

Both of these issues are fixed with this:

def update(
    self,
    db: Session,
    *,
    db_obj: ModelType,
    obj_in: Union[UpdateSchemaType, Dict[str, Any]]
) -> ModelType:
    if isinstance(obj_in, dict):
        update_data = obj_in
    else:
        update_data = obj_in.dict(exclude_unset=True)
    for field in update_data:
        setattr(db_obj, field, update_data[field])
    db.add(db_obj)
    db.commit()
    db.refresh(db_obj)
    return db_obj