Integrates Superset course-based permissions filters with Clickhouse xAPI data #6
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @pomegranited! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
and shifts Jinja filter to a separate module.
This role is assigned to all Open edX SSO users, and is used to mark row-based permissions.
so we can use it in the OARS plugin too
Also adds some TODO comments.
pomegranited
force-pushed
the
jill/superset-xapi-permissions
branch
from
867c4f1
to
945780a
Compare
pomegranited
changed the title
The user's role list isn't returned as an array of strings, it's an array of Roles, so updated the check accordingly.
| """ | ||
| Fetches the given user's access details from the Open edX User database | ||
| (since Open edX doesn't have an API for this). | ||
|
|
||
| NOTE: Open edX JWT seems to provide this info with the "profile" scope. |
There was a problem hiding this comment.
Wow is this all we're using the mysql db for?
There was a problem hiding this comment.
Yeup, that's it right now.
But it's likely that people will want to query things in MySQL to put in their charts, right? Which raises an operational note: does Tutor do read replicas yet? We don't really want people to have to slow down their LMS/Studio running expensive queries..
There was a problem hiding this comment.
Found the method that gives us the extra user data from the OAuth2 JWT token: openedx:auth_backends/backends.py
In that dict are "administrator" (is_staff) and "superuser" (is_superuser) values.
I think we'll need to get a JWT token in order to use this (we've currently just got an OAuth2 token): https://github.com/openedx/edx-platform/blob/master/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0003-use-jwt-as-oauth-tokens-remove-openid-connect.rst#requesting-jwt-tokens
There was a problem hiding this comment.
Ah neat, I'm not necessarily against using MySQL here but it would be nice to go through OAuth for all of this. I don't know that Tutor supports MySQL read replicas, I'd guess not since it might depend a lot on how / where you're hosting things. In theory we could use ClickHouse MySQL tables to connect to replicas if they exist and push the configuration there: https://clickhouse.com/docs/en/sql-reference/table-functions/mysql
I'm not sure that's the best option though. There's this task for figuring out whether it is feasible for permissions, though that seems unnecessary right now. It would probably be worth having for a lot of other things, though.
There was a problem hiding this comment.
Follow-up task: openedx/openedx-aspects#42
Not needed.
|
@pomegranited 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Updates Superset initialization and filters for openedx/openedx-aspects#30:
This role is used to mark the row-level security settings which restrict access to xAPI data by course, see Creates OARS datasets, charts, and dashboards in Superset openedx/tutor-contrib-aspects#3
can_view_coursesfilter to allow it to work on Clickhouse xAPI data.Miscellaneous fixes:
superset-appservice to justsuperset-- it's annoying, and keeps tripping me up.Testing instructions
See openedx/tutor-contrib-aspects#3.