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

Improve performance of updating with using update_field #71

Open
MAM-SYS opened this issue Jul 7, 2021 · 5 comments
Open

Improve performance of updating with using update_field #71

MAM-SYS opened this issue Jul 7, 2021 · 5 comments
Labels
good first issue Good for newcomers

Comments

@MAM-SYS
Copy link
Contributor

MAM-SYS commented Jul 7, 2021

Hello Friends
i was reviewing codes that i faced something

def register_user_with_email_and_password(email, password):

def change_user_password(user, password):

def open_auth_user_creator(email, first_name, last_name, profile_image):

in these methods we are changing profile data but we don't apply these changes to the database with user.profile.save() (we are just saving user changes)
is there any reason for this ?

@sbabashahi sbabashahi added the good first issue Good for newcomers label Jul 8, 2021
@sbabashahi
Copy link
Member

Hello @MAM-SYS

There is little point here

def save_user_profile(sender, instance, **kwargs):
you could read about django signals, if you are not familiar with this topic.

Although this issue is not a bug but we could have an improvement on our saving methods. Try to add update_fields. Also I'm not sure how it's work when you use it with signals. I change the issue title. Notice me if you can work on this performance issue.

@sbabashahi sbabashahi changed the title Updating Profile Model Improve performance of updating with using update_field Jul 8, 2021
@MAM-SYS
Copy link
Contributor Author

MAM-SYS commented Jul 10, 2021

@sbabashahi
Hello friend
apologize me
It was a bad bad bad mistake from me not to notice it
i was searching for a solution to pass update_field with signals.but could not find any good way
i have some idea but first you have to confirm it
i think we can remove the save_user_profile signal receiver and handling profile saving by profile.save(update_fields=[...]) method
some lines of code must change but it can improve the performance
if you decide....i will do it
thanks again for your response

@sbabashahi
Copy link
Member

sbabashahi commented Jul 10, 2021

@MAM-SYS your suggestion has an impact on all of our saves, we need to add profile.save every where we had user.save, I wsa thinking about a cheat :)), imagine you add update_fields of user in user.save(....) but before that you set an atribute on user lest call it _profile_update_fields then in save_user_profile we can extract this field and use it in instance.profile.save(....) long story short for example:

    ...
    user.set_password(password)
    user.profile.jwt_secret = utilities.uuid_str()
    user._profile_update_fields = ("jwt_secret",)
    user.save(update_fields=("password", ))
   ...

then in save_user_profile

    instance.profile.save(update_fields=getattr(instance, "_profile_update_field", None))

what do you think?

@MAM-SYS
Copy link
Contributor Author

MAM-SYS commented Jul 10, 2021

@sbabashahi
Yes, i got your idea
I was searching for many ways for this issue last 2 days
I think we have to change a lot of lines of code, whether your idea or mine
I was just thinking about getting ride of profile auto saving after each user.save() and save profile when we need with update_fields
I think it's worth it if we work on it
I trust your decision.everything you think is better, we do that
With respect...

@sbabashahi
Copy link
Member

I think we can continue with your idea, just test it for any side effect.

MAM-SYS added a commit to MAM-SYS/backend that referenced this issue Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants