docs: ADR to address Github project changes #193
Conversation
| Consequences | ||
| ************ | ||
|
|
||
| * `openedx-event-sink-clickhouse` and `platform_plugin_superset` will be consolidated into `openedx-aspects`_ and re-licensed to Apache 2. `openedx-event-sink-clickhouse` will be archived under the `openedx-unsupported`_ Github organization. |
There was a problem hiding this comment.
Could we see a sketch of the proposed file structure like we did for https://docsopenedxorg--193.org.readthedocs.build/projects/openedx-aspects/en/193/decisions/0008_project_structure.html#decisions , so we know where new plugins will live?
There was a problem hiding this comment.
One impact of this change which hasn't been mentioned: openedx-event-sink-clickhouse is currently a hard dependency for Aspects, but platform_plugin_superset is not. By consolidating both of those repos into this one, we've made openedx-aspects a hard dependency for Aspects, which means platform_plugin_superset gets installed whether users want it or not.
For optional plugins like this one, we should specify what convention will we use to enable/disable them in the LMS.
There was a problem hiding this comment.
The LMS integration works by using a filter which is configured using the following setting:
OPEN_EDX_FILTERS_CONFIG = {
"org.openedx.learning.instructor.dashboard.render.started.v1": {
"fail_silently": False,
"pipeline": [
"platform_plugin_superset.extensions.filters.AddSupersetTab",
]
},
}We need to make sure that tutor-contrib-aspects injects it into the settings without damaging what users have configured in their installations and make sure there is an up-to-date troubleshooting guide for Aspects
There was a problem hiding this comment.
I've updated to include the fact that this will just be a Django cookiecutter structure, which I don't think merits copying into this doc, but let me know if you feel differently. Also updated to include details on configuration and how each part can be turned on or off. Should be ready for re-review!
There was a problem hiding this comment.
I've updated to include the fact that this will just be a Django cookiecutter structure, which I don't think merits copying into this doc, but let me know if you feel differently.
Agreed.
Also updated to include details on configuration and how each part can be turned on or off.
Thank you! LGTM.
|
Does this mean we need to re-license https://github.com/eduNEXT/platform-plugin-superset/ to match the apache2 ? |
|
@felipemontoya I think that's a legal question, that we're not qualified to answer unfortunately. I've asked Axim legal whether we need anything else from you to accept this contribution, I'll let you know when I hear back. |
bmtcril
force-pushed
the
bmtcril/update_repo_scheme
branch
from
e5af431 to
5507844
Compare
No description provided.