add onupdate option in field simply #532

ponytailer · 2022-01-17T04:01:46Z

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

codecov-commenter · 2022-01-17T04:42:06Z

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%> (ø)`

ormar/fields/base.py

ormar/models/mixins/save_mixin.py

ponytailer · 2022-01-18T02:22:50Z

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 · 2022-01-18T09:09:12Z

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.

ormar/ormar/fields/foreign_key.py

Lines 185 to 196 in 8dc05d5

    
           def ForeignKey(  # type: ignore # noqa CFQ002 
        
               to: "ToType", 
        
               *, 
        
               name: str = None, 
        
               unique: bool = False, 
        
               nullable: bool = True, 
        
               related_name: str = None, 
        
               virtual: bool = False, 
        
               onupdate: str = None, 
        
               ondelete: str = None, 
        
               **kwargs: Any, 
        
           ) -> "T":

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

ormar/ormar/fields/base.py

Lines 242 to 252 in 8dc05d5

    
           constraints = [ 
        
               sqlalchemy.ForeignKey( 
        
                   con.reference, 
        
                   ondelete=con.ondelete, 
        
                   onupdate=con.onupdate, 
        
                   name=f"fk_{self.owner.Meta.tablename}_{self.to.Meta.tablename}" 
        
                   f"_{self.to.get_column_alias(self.to.Meta.pkname)}_{self.name}", 
        
               ) 
        
               for con in self.constraints 
        
           ] 
        
           return constraints

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 · 2022-01-18T09:13:32Z

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)

ormar/models/mixins/save_mixin.py

ormar/fields/base.py

ponytailer · 2022-01-18T11:01:46Z

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.

ormar/ormar/fields/foreign_key.py

Lines 185 to 196 in 8dc05d5

def ForeignKey( # type: ignore # noqa CFQ002

to: "ToType",

*,

name: str = None,

unique: bool = False,

nullable: bool = True,

related_name: str = None,

virtual: bool = False,

onupdate: str = None,

ondelete: str = None,

**kwargs: Any,

) -> "T":

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

ormar/ormar/fields/base.py

Lines 242 to 252 in 8dc05d5

constraints = [

sqlalchemy.ForeignKey(

con.reference,

ondelete=con.ondelete,

onupdate=con.onupdate,

name=f"fk_{self.owner.Meta.tablename}_{self.to.Meta.tablename}"

f"_{self.to.get_column_alias(self.to.Meta.pkname)}_{self.name}",

)

for con in self.constraints

]

return constraints

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.

ormar/models/model.py

ponytailer · 2022-01-19T02:21:46Z

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 · 2022-02-04T08:31:43Z

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 · 2022-02-04T08:37:00Z

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 · 2022-02-04T09:08:57Z

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 · 2022-02-08T02:28:31Z

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 · 2022-02-08T10:58:49Z

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 · 2022-02-08T11:06:14Z

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 · 2022-03-28T10:17:18Z

ormar/queryset/queryset.py

@@ -1111,7 +1114,15 @@ async def bulk_update(  # noqa:  CCR001

        columns = [self.model.get_column_alias(k) for k in columns]

+        # on_update_fields = {


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

vvanglro · 2023-04-07T12:08:58Z

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

ponytailer added 2 commits January 17, 2022 12:00

add onupdate option in field

2862e0f

with database

083780b

ponytailer added 5 commits January 17, 2022 12:46

fix lint

88cdf15

fix some lint and check the array in

00b4824

fix unnecessary lint

4357b6c

lazy doc

93edcbf

fix _columns args

5cff483

collerek reviewed Jan 17, 2022

View reviewed changes

fix

4db16eb

ponytailer requested a review from collerek January 18, 2022 02:36

collerek reviewed Jan 18, 2022

View reviewed changes

ormar/models/mixins/save_mixin.py Outdated Show resolved Hide resolved

ormar/models/mixins/save_mixin.py Outdated Show resolved Hide resolved

ponytailer added 2 commits January 18, 2022 17:26

fix

544b6aa

fix lint

2c0188d

collerek reviewed Jan 18, 2022

View reviewed changes

ormar/fields/base.py Outdated Show resolved Hide resolved

ponytailer added 2 commits January 18, 2022 18:59

fix naming

4721e9c

fix ut

30d0967

collerek reviewed Jan 18, 2022

View reviewed changes

ormar/models/model.py Show resolved Hide resolved

remove on bulk-update

60342b2

Merge branch 'master' into master

3916a69

fix

d30919c

ponytailer added 2 commits February 8, 2022 10:38

fix lint

362ba26

fix

5ee2196

collerek reviewed Mar 28, 2022

View reviewed changes

vvanglro mentioned this pull request Feb 27, 2024

feat: add on_update to the field #1273

Open

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add onupdate option in field simply #532

add onupdate option in field simply #532

ponytailer commented Jan 17, 2022 •

edited

Loading

codecov-commenter commented Jan 17, 2022 •

edited

Loading

ponytailer commented Jan 18, 2022 •

edited

Loading

collerek commented Jan 18, 2022

ponytailer commented Jan 18, 2022 •

edited

Loading

ponytailer commented Jan 18, 2022

ponytailer commented Jan 19, 2022 •

edited

Loading

collerek commented Feb 4, 2022

ponytailer commented Feb 4, 2022

collerek commented Feb 4, 2022

ponytailer commented Feb 8, 2022 •

edited

Loading

collerek commented Feb 8, 2022

ponytailer commented Feb 8, 2022

collerek Mar 28, 2022

vvanglro commented Apr 7, 2023 •

edited

Loading

		@@ -1111,7 +1114,15 @@ async def bulk_update( # noqa: CCR001

		columns = [self.model.get_column_alias(k) for k in columns]

		# on_update_fields = {

add onupdate option in field simply #532

add onupdate option in field simply #532

Conversation

ponytailer commented Jan 17, 2022 • edited Loading

codecov-commenter commented Jan 17, 2022 • edited Loading

Codecov Report

ponytailer commented Jan 18, 2022 • edited Loading

collerek commented Jan 18, 2022

ponytailer commented Jan 18, 2022 • edited Loading

ponytailer commented Jan 18, 2022

ponytailer commented Jan 19, 2022 • edited Loading

collerek commented Feb 4, 2022

ponytailer commented Feb 4, 2022

collerek commented Feb 4, 2022

ponytailer commented Feb 8, 2022 • edited Loading

collerek commented Feb 8, 2022

ponytailer commented Feb 8, 2022

collerek Mar 28, 2022

Choose a reason for hiding this comment

vvanglro commented Apr 7, 2023 • edited Loading

ponytailer commented Jan 17, 2022 •

edited

Loading

codecov-commenter commented Jan 17, 2022 •

edited

Loading

ponytailer commented Jan 18, 2022 •

edited

Loading

ponytailer commented Jan 18, 2022 •

edited

Loading

ponytailer commented Jan 19, 2022 •

edited

Loading

ponytailer commented Feb 8, 2022 •

edited

Loading

vvanglro commented Apr 7, 2023 •

edited

Loading