-
Notifications
You must be signed in to change notification settings - Fork 4.2k
loc mapper memoize #1954
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
loc mapper memoize #1954
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pythonic last ele mechanism. Not important.
Conflicts: cms/djangoapps/auth/authz.py
STUD-1039
|
@chrisndodge Do you have time to review this? I'm don't necessarily think I am the best person.... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the syntax for name__in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
field__in = listvalues translates to a sql where field in listvalues Something @cpennington pointed out to me and correctly suggested we use.
|
OK, there's quite a bit of stuff I'm unfamiliar with, but I understand enough to give it a +1 |
STUD-1048
cms/djangoapps/auth/authz.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user.groups.remove(*user.groups.filter(name__in=groupnames))?
|
@cpennington I configured a different cache and used the remove syntax. I assume this is ready to go now? @e0d this looks for a cache named 'loc_cache' and if it doesn't find it uses 'default'. 'loc_cache' can be a simple local memory cache, imho, but could be a general memcache. I assume a local mem cache performs a lot better than a shared memcache. |
|
@e0d @dmitchell If it's a local memory cache, then it's going to be important to get the arguments right: https://docs.djangoproject.com/en/1.4/topics/cache/#cache-arguments In particular, MAX_ENTRIES is going to have to be large enough that we get benefit from the caching, and small enough that we don't use all the memory on the box. |
STUD-1048
Note, this PR has the stuff I put into the hotfixes as I was monotonically building on those but they haven't been merged to master yet. If they get merged from release before this PR, I'll rebase this one.
So, you may want to just review the last commit as the others have been reviewed in PR 1902 and 1937.
@singingwolfboy @cahrens Please review or delegate (other potential reviewers include @chrisndodge @cpennington ...)