Skip to content
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

[BD-32] feat: Add Hooks Extension Framework tooling #110

Closed

Conversation

mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented Mar 24, 2021

Description:

This PR adds a new module for the tools needed by the Hooks Extension Framework to execute filters. This project is described in more depth in the OEP-50.

This tooling will allow us to extend the platform by adding triggers in specific places, those triggers will be able to execute a list of filters previously configured.

For that to happen, we are introducing two functions called triggers_filter. When called, these functions will execute a Pipeline with the list of functions specified by the configuration. This pipeline is defined in ADR Hooks tooling: pipeline

Now, this configuration as indicated in ADR: Configuration for the Hook Extension Framework is defined using Django settings and looks like this (extended format):

HOOKS_EXTENSION_CONFIG = {
      "openedx.lms.module.some_trigger.filter.v1": {
               "pipeline": [
                     "somelibrary.filters.1st_function",
                     "other_library.filters.2nd_function"
                     ],
     },

Pipeline is the list of functions that the pipeline runner will execute, and async indicates how the pipeline is going to run, asynchronous or synchronous. Please be aware that this PR does not add the asynchronous feature, only the synchronous, that feature is meant to be added in another PR to reduce complexity.

Please, check out the ADR where the other configuration formats are defined.

JIRA:

XXX-XXXX

Dependencies:

List dependencies on other outstanding PRs, issues, etc.

Merge deadline:

None

Testing instructions:

  1. Run make lms-shell
  2. Inside the shell run:
    pip install -e git+https://github.com/edx/edx-django-utils.git@MJG/hooks_pipeline_tool#egg=edx_django_utils
  3. Run pip install -e https://github.com/eduNEXT/openedx-basic-hooks.git, you can also use your own test library. Here are defined the functions that will be executed by a trigger.
  4. Here's a quick and dirty way to create and use triggers on edx-platform. We defined three triggers and the configuration needed to use this framework. PENDING: add link to the PR against edx-platform that includes the first implementation of a trigger.
  5. (Optional) If you're using our demo and sample openedx-hooks-library, you have a trigger_filter ready to be used. What you'll have to do is execute the logic that the hook extended, for example:

Reviewers:

  • tag reviewer

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPi after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns:

  • Is this the best place for this feature? This concern is raised when reading:

Note that some utilities may warrant their own repository. A judgement call needs to be made as to whether code properly belongs here or not. Please review with the Architecture Team if you have any questions.

  • Will there be a problem with adding celery? Adding it will cause other applications to crash?

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 24, 2021
@openedx-webhooks
Copy link

openedx-webhooks commented Mar 24, 2021

Thanks for the pull request, @mariajgrimaldi! I've created BLENDED-806 to keep track of it in Jira.

When this pull request is ready, tag your edX technical lead.

@mariajgrimaldi mariajgrimaldi changed the title feat: Add Hooks Extension Framework tooling [BD-32] feat: Add Hooks Extension Framework tooling Mar 25, 2021
@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program open-source-contribution PR author is not from Axim or 2U and removed open-source-contribution PR author is not from Axim or 2U blended PR is managed through 2U's blended developmnt program labels Mar 25, 2021
@mariajgrimaldi mariajgrimaldi marked this pull request as ready for review March 25, 2021 15:20
@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program needs triage open-source-contribution PR author is not from Axim or 2U and removed open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. blended PR is managed through 2U's blended developmnt program labels Mar 25, 2021
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/hooks_pipeline_tool branch 4 times, most recently from dc78c58 to b50a5f0 Compare March 26, 2021 16:34
* Pipeline runner for actions and filters
* Triggers for actions and filters
@mariajgrimaldi
Copy link
Member Author

Hey! I made a PR showing how it'll look adding the async feature -if we decide that this repository is the best place for Hooks Extension Framework-:
#111

@mariajgrimaldi
Copy link
Member Author

Closing PR following recommendations made on this post: https://discuss.openedx.org/t/configuration-for-the-hooks-extension-framework/4527
From now on, this PR is going to be found here: https://github.com/eduNEXT/openedx-filters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blended PR is managed through 2U's blended developmnt program rejected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants