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

feat: add Hooks Extension Framework tooling for filters #2 #1

Merged
merged 1 commit into from
May 13, 2021

Conversation

mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented Apr 21, 2021

Description:

This PR adds the tooling needed by the Hooks Extension Framework to execute filters. This project is described in more depth in the OEP-50.

The tooling consists of a pipeline (Pipeline tooling ADR) that executes a list of functions pre-configured that are associated with the filter via Django settings (Filters configuration ADR).

Dependencies:
The architectural design records where the configuration and pipeline design used by the tooling are defined in Pull Request #2.

Testing instructions:

  1. Run make lms-shell
  2. Inside the shell run:
    pip install -e git+https://github.com/eduNEXT/openedx-filters.git@MJG/filter_tooling#egg=filter_tooling
  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 filters on edx-platform. In the example we are using pre_register_filter that's soon to be added (PR pending).
  5. (Optional) If you're using our demo and sample openedx-base-hooks, you can use our pre_register_filter. What you'll have to do is execute the logic that the filter extended, for example:
  • Go to register
  • Enter data for registration and in the field Year of birth add 2020 (or any year after 2000).
  • Then, an error will be displayed caused by the HookFilterException raised by our filter function defined in this location

Reviewers:

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)

Copy link
Contributor

@morenol morenol left a comment

Choose a reason for hiding this comment

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

This looks great! I added a couple of comments

openedx_filters/__init__.py Outdated Show resolved Hide resolved
openedx_filters/exceptions.py Outdated Show resolved Hide resolved
openedx_filters/exceptions.py Outdated Show resolved Hide resolved
openedx_filters/pipeline.py Outdated Show resolved Hide resolved
openedx_filters/tests/test_pipeline.py Outdated Show resolved Hide resolved
openedx_filters/tests/test_pipeline.py Outdated Show resolved Hide resolved
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/filter_tooling branch 2 times, most recently from 4b13e22 to e0f1c15 Compare May 4, 2021 18:42
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/fix_tests_config branch from 30c2d1e to 82cc9ab Compare May 4, 2021 18:47
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/filter_tooling branch 3 times, most recently from 47afaec to b415432 Compare May 4, 2021 19:44
@mariajgrimaldi mariajgrimaldi changed the base branch from MJG/fix_tests_config to main May 4, 2021 19:46
Copy link
Member

@felipemontoya felipemontoya left a comment

Choose a reason for hiding this comment

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

I tested this with different filter definitions, different pipeline configurations and different logic for raising exceptions and it performed as we have so long discussed via all the channels.

Thanks @mariajgrimaldi, I think we are ready to merge

@felipemontoya
Copy link
Member

Please squash the 10 commits before merging

This PR adds the tooling needed to execute filters, a pipeline deeply inspired
in the TPA Pipeline from social core project.
@mariajgrimaldi mariajgrimaldi merged commit 88ebc91 into main May 13, 2021
@mariajgrimaldi mariajgrimaldi deleted the MJG/filter_tooling branch March 28, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants