add onupdate option in field simply

[ponytailer]

• support option onupdate in field simply
• fix lint
• check the args in bulk-create

[codecov-commenter]

Codecov Report

Merging #532 (5ee2196) into master (4ed267e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #532   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          185       186    +1     
  Lines        15298     15368   +70     
=========================================
+ Hits         15298     15368   +70     

Impacted Files	Coverage Δ
ormar/models/newbasemodel.py	100.00% <ø> (ø)
...test_model_methods/test_populate_default_values.py	100.00% <ø> (ø)
ormar/fields/base.py	100.00% <100.00%> (ø)
ormar/models/metaclass.py	100.00% <100.00%> (ø)
ormar/models/mixins/save_mixin.py	100.00% <100.00%> (ø)
ormar/models/model.py	100.00% <100.00%> (ø)
ormar/queryset/queryset.py	100.00% <100.00%> (ø)
...est_model_methods/test_populate_onupdate_values.py	100.00% <100.00%> (ø)
tests/test_queries/test_queryset_level_methods.py	100.00% <100.00%> (ø)

[ponytailer]

Cascade should be another option in field ? @collerek

The onupdate don't handle the relations.

such like the sqlalchemy, and the cascade should define in the database.

book = ormar.ForiegnKey(Book, cascade=CASCADE.DELETE)

[collerek]

Cascade should be another option in field ? @collerek

The onupdate don't handle the relations.

such like the sqlalchemy, and the cascade should define in the database.

book = ormar.ForiegnKey(Book, cascade=CASCADE.DELETE)

Yes, it does.
There are already onupdate and ondelete, but only for foreign keys.

https://github.com/collerek/ormar/blob/8dc05d5cf48469f6285e88ab87d38b57c678976f/ormar/fields/foreign_key.py#L185-L196

It's used to populate onupdate and ondelete foreign key options in sqlalchemy foreign key:

https://github.com/collerek/ormar/blob/8dc05d5cf48469f6285e88ab87d38b57c678976f/ormar/fields/base.py#L242-L252

Where you pass a string according to sqlalchemy core specification:
https://docs.sqlalchemy.org/en/14/core/constraints.html#on-update-and-on-delete

[ponytailer]

That's in foreignKey only.

when the model was updated, something to do auto, such as the updated_time, that's my purpose.

otherwise, I have to define the code in model_cls

async def update(self,  **kwargs):
     kwargs["updated_time"] = get_ts()
     super().update(**kwargs)

[ponytailer]

Cascade should be another option in field ? @collerek
The onupdate don't handle the relations.
such like the sqlalchemy, and the cascade should define in the database.

book = ormar.ForiegnKey(Book, cascade=CASCADE.DELETE)

Yes, it does. There are already onupdate and ondelete, but only for foreign keys.

https://github.com/collerek/ormar/blob/8dc05d5cf48469f6285e88ab87d38b57c678976f/ormar/fields/foreign_key.py#L185-L196

It's used to populate onupdate and ondelete foreign key options in sqlalchemy foreign key:

https://github.com/collerek/ormar/blob/8dc05d5cf48469f6285e88ab87d38b57c678976f/ormar/fields/base.py#L242-L252

Where you pass a string according to sqlalchemy core specification: https://docs.sqlalchemy.org/en/14/core/constraints.html#on-update-and-on-delete

thanks for your suggestions.

[ponytailer]

This pr only include the update, not the bulk-update.
because

        for obj in objects:
            # when the obj.__setattr__, should be dirty for column
            # only load the kv from dirty fields, not all.
            new_kwargs = obj.dict()

Please review the #538 first, the orjson.dumps in bulk-create need fix.

[collerek]

Can you restore/uncomment bulk_update changes?
For now let's keep the changes logic as you proposed, without keeping status of each field (dirty or not)

[ponytailer]

Can you restore/uncomment bulk_update changes?

For now let's keep the changes logic as you proposed, without keeping status of each field (dirty or not)

sorry. I'm on vacation now. Maybe I can create the new pr for bulk-update in a few days.

[collerek]

Can you restore/uncomment bulk_update changes?
For now let's keep the changes logic as you proposed, without keeping status of each field (dirty or not)

sorry. I'm on vacation now. Maybe I can create the new pr for bulk-update in a few days.

Ahh of course. Happy Lunar New Year! 🥳 🎉

No worries it can wait for your come back :)

[ponytailer]

Can you restore/uncomment bulk_update changes?
For now let's keep the changes logic as you proposed, without keeping status of each field (dirty or not)

sorry. I'm on vacation now. Maybe I can create the new pr for bulk-update in a few days.

Ahh of course. Happy Lunar New Year! 🥳 🎉

No worries it can wait for your come back :)

Sorry, I forgot something. In bulk-update, I don't know what field was updated, so, on_update don't work.
This pr only add on_update work in update method.

Next pr will add the dirty-columns, it can support bulk-update.

If this pr has not any other problems, maybe merge it now.

[collerek]

Before you implement something describe what you want to do.
I told you that keeping the dirty status on fields might have a big performance influence and can be unreliable.

[ponytailer]

Before you implement something describe what you want to do. I told you that keeping the dirty status on fields might have a big performance influence and can be unreliable.

Thanks for your advice !

[collerek]

please either update the code or remove it -> we don't want commented out code in the code base

[vvanglro]

Hi guys why is this PR not merged, I wish there was an onupdate like sqlalchemy.
column-insert-update-defaults