gtalarico / pyairtable

Python Api Client for Airtable
https://pyairtable.readthedocs.io
MIT License
765 stars 138 forks source link

Proposing a different take on ORM solution #248

Closed thismlguy closed 1 year ago

thismlguy commented 1 year ago

Hello, I have been using the API for a while now and have struggled with the ORM solution. My understanding is that right now all create and load operations make one call per row, which is turning out to be really slow. I wanna propose an alternative ORM implementation, something along the lines of what I've been using as an internal module. It leverages some capabilities built in pydantic basemodels.

I've only implemented text and date fields for now for illustration.

@mesozoic @gtalarico - what do you think about this?

mesozoic commented 1 year ago

What does "all create and load operations make one call per row" mean? What capabilities does pydantic give us that we don't have today?

There are some open issues with the ORM (like #188 and #190) and some missing field types, and I'm working on those now. But I don't know how to weigh the value of a total rewrite that might significantly change how implementers use and extend the ORM module.

gtalarico commented 1 year ago

Seems like there are two issues to unpack here:

  1. should we adopt pydantic? I like pydantic but I don't understand the use case/advantages it would provide

  2. "make one call per row" > batch operations? If I understand correctly, you want to perform operations on multiple instances a single api batch operation? This could be done seamlessly w/ current ORM implementation by adding batch methods to model class e.g.

class Model:
   @classmethod
   def batch_save(cls, instances: List[T]):
       # similar to save(), but use batch methods to call create/update

model_instances = [Model.from_record(record) for record in records]
Model.batch_save(model_instances)
# where batch_save() is a class method that will call underlaying `cls.get_table().batch_*` methods

What do you think?

mesozoic commented 1 year ago

Thanks for bringing up this suggestion! It spurred me to go learn a lot more about pydantic. I've got a pull request (#274) to add batch save/delete operations to the ORM module, which looks like a clear gap.

Despite having found a few other use cases for pydantic, some of which are already merged, I don't see what other features we'd gain by changing the orm.Model base class to be derived from pydantic.BaseModel, and I can think of a few ways it could break backwards-compatibility with existing code. If you can think of other capabilities the ORM module should support, let's open new issues for those specific features. For now I'm going to close this thread.