jobguywork / backend

Job Guy Backend
https://jobguy.work/donate
MIT License
215 stars 46 forks source link

Improve performance of updating with using update_field #71

Open MAM-SYS opened 3 years ago

MAM-SYS commented 3 years ago

Hello Friends i was reviewing codes that i faced something https://github.com/jobguywork/backend/blob/5ef2dca266aaf6f2000e0765894045642e48ef98/authnz/transactions.py#L9 https://github.com/jobguywork/backend/blob/5ef2dca266aaf6f2000e0765894045642e48ef98/authnz/transactions.py#L19 https://github.com/jobguywork/backend/blob/5ef2dca266aaf6f2000e0765894045642e48ef98/authnz/transactions.py#L26 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 commented 3 years ago

Hello @MAM-SYS

There is little point here https://github.com/jobguywork/backend/blob/5ef2dca266aaf6f2000e0765894045642e48ef98/authnz/models.py#L47 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.

MAM-SYS commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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