Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for update_fields to choose 'all' #35

Open
arady22 opened this issue Sep 20, 2022 · 4 comments
Open

Add option for update_fields to choose 'all' #35

arady22 opened this issue Sep 20, 2022 · 4 comments

Comments

@arady22
Copy link

arady22 commented Sep 20, 2022

Title
when calling the function to update all the fields instead of calling

model_field_names = [field.name for field in model._meta.get_fields()]
model.objects.bulk_update_or_create(bulk_data, model_field_names, match_field='object_guid')

we call
model.objects.bulk_update_or_create(bulk_data, 'all', match_field='object_guid')

@fopina
Copy link
Owner

fopina commented Sep 28, 2022

Curious to understand why would you update all fields (apart from developer laziness).

This module is meant to bulk update (as in update many records). If you have to specify the fields that you're changing when modifying the objects, why would you NOT specify the fields in the update and incur in bigger data transfer than required?

@arady22
Copy link
Author

arady22 commented Sep 29, 2022

Long story short we update/sync records coming from JSON API so we have no idea which records and fields really changed so we choose to update all.

btw we have to exclude primary keys from the model_field_names.

@fopina
Copy link
Owner

fopina commented Feb 17, 2023

The main reason I don't think it's good to support all fields is to avoid mistakes.

Imagine you have a model that fully maps to a JSON you consume. So you use all.

At some point, you add a new field to the model to be manually managed, it's not part of the parsed JSON.

If you don't remember to replace "all" with the list of all fields except that one, this will clear that field (because the objects from the JSON file always leave it empty).

I'll leave this issue open to document a workaround to pass all fields (ie: Model.get_fields()) but leaving it up to consumer to abuse that.

@housUnus
Copy link

The *all options is helpful in case we have a model with many fields, it's tidy work to mention all the fields of the model, when it should select all and exclude the match_field, which BTW it'll be better if it is specified as a list 'cause when I receive the object without an id I would like to compare the title + another field to ensure the uniqueness then update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants