Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

feat: adds superset docker services and open edx integrations #1

Merged
merged 4 commits into from
Jan 17, 2023

Conversation

pomegranited
Copy link
Contributor

@pomegranited pomegranited commented Nov 30, 2022

This plugin runs Superset in Tutor, with the following customizations:

  • installs python packages to support mysql and OAuth2
  • configures SSO from LMS to Superset
  • adds custom oauth2 manager which uses LMS APIs and the mysql database to determine user permission levels.
  • adds custom course permissions plugin for use with SQL queries.

Testing instructions

See updated README.rst

Author Notes & Concerns

  1. Should we use the openedx redis instance instead of running our own here?
    A: Yes
  2. Can/should we use the openedx mysql database for superset instead of running a separate postgres db?
    I tried this, but wasn't able to work out how to get superset's db migrations working.
  3. I think this needs a follow-up PR, which updates superset init script to use the Superset API to create the OpenedX dataset + chart, and set up roles and row-level security. But I think this can wait until we know exactly how we're going to use Superset with Open edX.

Copy link
Collaborator

@bmtcril bmtcril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading through it this looks great and solves a number of issues I was worried about during my small Superset test! I have been running long tests all week, but hope to fire this up on Monday and test it out, thanks so much!

README.rst Outdated Show resolved Hide resolved
tutorsuperset/plugin.py Outdated Show resolved Hide resolved
OPENEDX_COURSES_LIST_URL: {{ SUPERSET_OPENEDX_COURSES_LIST_URL }}
SECRET_KEY: {{ SUPERSET_SECRET_KEY }}
PYTHONPATH: /app/pythonpath:/app/docker/pythonpath_dev
REDIS_HOST: superset_cache
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you asked this elsewhere, but it would be cool if we could use the same redis as LMS for sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was easy to do once I figured out the right init scripts! This is now using Open edX's redis for caching, and MySQL for the backend database instead of postgres.

tutorsuperset/plugin.py Outdated Show resolved Hide resolved
tutorsuperset/templates/superset/tasks/init-superset.sh Outdated Show resolved Hide resolved
tutorsuperset/plugin.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@bmtcril bmtcril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is great, and I am hoping folks can use it to work with permissions while we work on getting ClickHouse set up. I think I documented the few issues I ran into, most of which were my own fault.

One other random things is that I have a tutor_nightly_dev-superset-1 container which is always in "restarting" state. No idea what I may have done to cause that, but just want to make sure you don't see the same thing.

Not something for this PR, but just to be aware of - based on some of the use cases from the community my next round of thoughts on permissions and dashboards are here: openedx/openedx-aspects#28

I'd be happy to thumb this if the docs get updated and examples removed, then we can use it a little more easily as grounds for further investigation!

Copy link
Collaborator

@bmtcril bmtcril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

tutorsuperset/plugin.py Outdated Show resolved Hide resolved
* install custom requirements for mysql and OAuth2
* install custom oauth2 manager and course permissions plugin, which
  uses the Open edX mysql db to fetch user staff/superuser status
* init OAuth2 application on LMS
* replaces deprecated Tutor init commands with latest version
Also:
* After Open edX SSO is enabled, there's no way to login as the Superset
  Admin user, so there's no need to create one.
* pins superset version
instead of superset's own redis + postgres.

Also:
* use redis for all superset caches
* tweaks some comments, nothing huge
@pomegranited
Copy link
Contributor Author

@bmtcril I believe I've addressed all the bugs you raised, and the wishlist items:

One other random things is that I have a tutor_nightly_dev-superset-1 container which is always in "restarting" state. No idea what I may have done to cause that, but just want to make sure you don't see the same thing.

This was caused by an unnecessary service I had wrapping all the superset services. It's been removed now, so you shouldn't see this anymore (though you may have to fully stop Tutor and relaunch).

Not something for this PR, but just to be aware of - based on some of the use cases from the community my next round of thoughts on permissions and dashboards are here: openedx/openedx-aspects#28

Yep yep, I'll address those with my next PR.

I'd be happy to thumb this if the docs get updated and examples removed, then we can use it a little more easily as grounds for further investigation!

Done! Thanks for your approval! I'm going to merge this, and will iterate with the next issue.

@pomegranited pomegranited merged commit 85b49d5 into main Jan 17, 2023
@pomegranited pomegranited deleted the jill/install-superset branch January 17, 2023 10:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants