feat: Improve handling of deep taxonomies (perf + limits + bugfixes)#511
feat: Improve handling of deep taxonomies (perf + limits + bugfixes)#511bradenmacdonald wants to merge 3 commits intomainfrom
Conversation
|
Thanks for the pull request, @bradenmacdonald! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
796a8b4 to
e74f5f0
Compare
fix: API results are now correct regardless of tag depth feat: refuse to create tags deeper than TAXONOMY_MAX_DEPTH perf: make "depth" and "lineage" concrete
e74f5f0 to
a74e685
Compare
This is a fix for openedx/modular-learning#257 .
In short: the API was very inconsistent with how it handles deeply-nested tags, and arguably the behavior was buggy.
Approach: This PR updates the
Tagdata model to storedepthandlineageas columns, rather than computing them dynamically. Then, I rewrote all the queries to support unlimited tag depth. With thedepthandlineagecolumns available, we can perform all the same queries very efficiently without having to hard-code things likeparent__parent__parent__...that assume a certain depth limit. Now all the API methods work with an unlimited tag depth.Actual depth limit: This PR also clarifies the definition of
TAXONOMY_MAX_DEPTHand actually enforces it to limit the allowed depth to six levels. (Before this, no limit was enforced when creating tags. A limit of 3 levels was enforced when reading tags from multiple levels at once, but it didn't work well below the root.)Perfomance: pretty much on par with the
mainbranch in every way I could measure. Significantly faster than my CTE approach #510 .AI disclosure: Claude assisted with this PR.