Models with _auto_sync are not saved on instantiation

[andrewthetechie]

Describe the bug
Originally reported by @david-whalstedt in #379

Furthermore I have noticed that if you create an instance with _auto_sync on (default), it is not saved in Redis until you assign some field to it after creation. I

To Reproduce
Steps to reproduce the behavior:

1. Create a model and make sure _auto_sync is true
2. Instantiate an instance of the model
3. Confirm it is not saved to Redis

[andrewthetechie]

This is an expected outcome with how the code currently works

_auto_sync is expected to sync changes to a model that has already been saved to Redis. For example, if you retrieve a model from Redis and update one of its fields, then _auto_sync will save that field change into Redis.

Perhaps a new _auto_save option could be used to save a model on instantiation if that's the desired behavior.

[andrewthetechie]

Docs could also be improved to make it clear how _auto_sync works

[david-wahlstedt]

In those cases we have chosen _auto_sync to be on, don't we always want to have it in Redis when created? Can you give an example of when it wouldn't be preferable?

[andrewthetechie]

Here is what I'm thinking as a fix for your request @david-wahlstedt #396

I'm open to feedback if this doesn't work for you.

[david-wahlstedt]

Hi Andrew, I can't find this comment in the web gui, only here in this email, so I reply here: Your change looks fine to me. I added a comment that maybe you can try the syncer.sync(foo()) for calling async foo from a sync context. For my own usage: I will use the non-autosave, because I work on a solution where I want a "counter" feature. ``` class MyModel(Model): _primary_key_field: str = "key" key: str value: Value timestamp: datetime.datetime # or maybe an ISO Z str count: int # "automatically" incremented when interesting parts of the record gets updated ``` and whenever a record is saved and the value field is changed, the counter should increment. If other fields are changed the counter should not increment. Maye there this could be a feature request? Maybe I should use the timestamp and have it mandatory updated in order to update the value field. Then I can also count updates that only updates the timestamp, but actually writes the old value to `value` again. Anyway, since I want to be able to trigger some things to happen when certain updates are performed, it feels safer to have explicit save operations. Sorry if my reviewing response is slow, it is partly because I find it hard to navigate and find the code changes pending for my review in the github web GUI. Best regards, David

…

On Fri, Dec 16, 2022 at 11:56 PM Andrew ***@***.***> wrote: The _auto_sync feature is also buggy with how it handles async. https://github.com/andrewthetechie/pydantic-aioredis/blob/main/pydantic_aioredis/model.py#L127-L143 I get why I wrote this, but that code is just not good. It results in lots of problems where the event loop exits weirdly, nested async is just not a good plan. Unfortunately, its not possible to make *setattr* async, so it may be better to remove auto_sync altogether. — Reply to this email directly, view it on GitHub <#380 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AP2Q2JOQCX345RGVOK3TVQDWNTXP7ANCNFSM6AAAAAAS6WIG4Y> . You are receiving this because you commented.Message ID: ***@***.***>

[andrewthetechie]

Thanks for checking it out! I appreciate your feedback.

No worries on response time, you're plenty fast!

[andrewthetechie]

Closed my previous PR and refactored as #401

This adds a new parameter to models, _auto_save, that will save a model to Redis on instantiation. It also adds documentation on how to use _auto_save, _auto_sync, and the new update()