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

Remove easy-rules #540

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

willmostly
Copy link
Contributor

@willmostly willmostly commented Oct 30, 2024

Description

Remove easy-rules and use mvel directly for rule evaluation This PR closes #527. The existing rule format is retained, with the exception that easy-rules style

Additional context and related issues

Trino Gateway currently leverages easy-rules to evaluate routing rules against an http request. This PR

  • removes easy-rules due to Remove easyrules #527
  • provides a framework for managing and evaluating rules
  • replicate existing easy-rules functionality: routing rules with local file based storage and the MVEL engine for evaluation

There is ongoing discussion on which evaluation engine[s] to support. The abstract RoutingRule and RulesRoutingGroupSelector classes are intended to provide a framework to facilitate migrating to a new engine when a decision is made.

The MVELRoutingRule and MVELFileRoutingGroupSelector replicate the existing easy-rules functionality, minus composite rules. Easy-rules uses mvel as the execution engine, so rule condition and action syntax does not need to be migrated. This PR drops support for composite rules, which may be a breaking change for some users.

Database storage for MVEL rules will be added as a follow up.

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x ) Release notes are required, with the following suggested text:
Remove easy-rules library. Breaking change: composite rules are deprecated

@cla-bot cla-bot bot added the cla-signed label Oct 30, 2024
@mosabua
Copy link
Member

mosabua commented Oct 30, 2024

Have not looked but removing one layer of indirection that just adds security issues and a bunch of syntax sugar seems like a good idea.

@mosabua
Copy link
Member

mosabua commented Oct 31, 2024

Can you ask in the maintainers group about any potential suggestion or preference for a rules engine

@willmostly willmostly force-pushed the will/mvel-rules branch 2 times, most recently from 2c476fc to e398d98 Compare October 31, 2024 00:32
@@ -67,7 +67,6 @@ static Stream<String> provideRoutingRuleConfigFiles()
String rulesDir = "src/test/resources/rules/";
return Stream.of(
rulesDir + "routing_rules_atomic.yml",
rulesDir + "routing_rules_composite.yml",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR removes support for composite rules

@willmostly willmostly force-pushed the will/mvel-rules branch 5 times, most recently from de8b88e to d2d6bfa Compare November 1, 2024 17:10
@willmostly willmostly changed the title Use MVEL for rule evaluation (WIP - Not for review) Remove easy-rules Nov 1, 2024
@willmostly willmostly requested review from mosabua, ebyhr and oneonestar and removed request for mosabua and ebyhr November 4, 2024 16:54
@mosabua
Copy link
Member

mosabua commented Nov 4, 2024

We should get that into 13 imho .. I wont be able to review this week before Friday

Co-Authored-By: Prakhar Sapre <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Remove easyrules
2 participants