Fixes for permissions cache and localization issues #489
Conversation
| @@ -209,7 +230,9 @@ def get_courses(self, username, permission="staff", next_url=None): | |||
| username=username, permission=permission | |||
| ) | |||
| url = next_url or courses_url | |||
| response = oauth_remote.get(url, token=token).json() | |||
| resp = oauth_remote.get(url, token=token) | |||
| resp.raise_for_status() | |||
There was a problem hiding this comment.
Previously we ignored errors here which limited logging things like the LMS 429 rate limit errors, this will now raise them for easier debugging.
| REDIS_RESULTS_DB = get_env_variable("REDIS_RESULTS_DB", "1") | ||
| REDIS_CELERY_DB = get_env_variable("REDIS_CELERY_DB", "3") | ||
| REDIS_RESULTS_DB = get_env_variable("REDIS_RESULTS_DB", "4") | ||
| REDIS_CACHE_DB = get_env_variable("REDIS_CACHE_DB", "5") |
There was a problem hiding this comment.
Moving these to some other databases in redis so they are easier to identify. Previously they shared namespace with LMS which made debugging challenging and increased the possibility of name collisions.
| # should the timeout be reset when retrieving a cached value | ||
| "REFRESH_TIMEOUT_ON_RETRIEVAL": True, | ||
| "REFRESH_TIMEOUT_ON_RETRIEVAL": False, |
There was a problem hiding this comment.
Bumping down the time and forcing these to refresh now, there could be some confusing artifacts when the filters got cached but a user's permissions had changed. This cuts down the possible length of time for that state to 20 mins. Artifacts were things like problem and video names appearing selected in the filters when the user no longer had access to them, though the charts do the right thing and do not display the data.
|
|
||
| return memoized_func(key="{username}", cache=cache_manager.cache)(can_view_courses)(*args, **kwargs) | ||
| kwargs["cache_timeout"] = {{ SUPERSET_USER_PERMISSIONS_CACHE_TIMEOUT }} |
There was a problem hiding this comment.
This was the actual bug in caching user permissions. Superset docstring says the memoized_func defaults to 300 secs, but it is actually 0 (indefinite). Passing in an explicit timeout fixes it.
| @@ -1,6 +1,6 @@ | |||
| _file_name: Instructor_Dashboard.yaml | |||
| _roles: | |||
| - {{ SUPERSET_ROLES_MAPPING.instructor }} | |||
| - "{{ SUPERSET_ROLES_MAPPING.instructor }}" | |||
| css: '' | |||
There was a problem hiding this comment.
For the localization process to work now we need to wrap all curly braces in double quotes. This still needs testing on the translations_pull side once the existing translations are updated, but I think it will work.
tutoraspects/templates/aspects/apps/superset/pythonpath/superset_config.py
Show resolved
Hide resolved
During the upgrade to Superset 3 the course cache accidentally became permanent due to incorrect Superset documentation. This cleans up caching of permissions in general as well as fixing some small localization bugs found in testing.
bmtcril
force-pushed
the
bmtcril/fix_permissions_cache
branch
from
636649b to
4d793e3
Compare
Fixes: #406