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

replaced govaluate with expr #2911

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AndrewChubatiuk
Copy link

Removed govaluate, which is not maintained and left expr only instead

@AndrewChubatiuk AndrewChubatiuk force-pushed the replace-govaluate-with-expr branch 6 times, most recently from 20c32ed to 8215f19 Compare December 11, 2023 15:57
@@ -65,7 +66,7 @@ func NewJetstreamTriggerConn(conn *jetstreambase.JetstreamConnection,
recentMsgsByTime: make([]*msg, 0)}
connection.Logger = connection.Logger.With("triggerName", connection.triggerName, "sensorName", connection.sensorName)

connection.evaluableExpression, err = govaluate.NewEvaluableExpression(strings.ReplaceAll(dependencyExpression, "-", "\\-"))
connection.evaluableExpression, err = expr.Compile(dependencyExpression)
Copy link
Member

Choose a reason for hiding this comment

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

Don't need to escape it any more?

Copy link
Author

Choose a reason for hiding this comment

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

wanted to deprecate - in naming, cause it limits subtract operations, reverted this change for now. want to add abilities to set priorities later in trigger conditions

@@ -7,17 +7,17 @@ spec:
serviceAccountName: operate-workflow-sa
# defines list of all events sensor will accept
dependencies:
- name: test-dep
- name: TestDep
Copy link
Member

Choose a reason for hiding this comment

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

Curious, what's the reason to change?

Copy link
Author

Choose a reason for hiding this comment

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

reverted it as well, not relevant anymore

@AndrewChubatiuk AndrewChubatiuk force-pushed the replace-govaluate-with-expr branch 5 times, most recently from 527b927 to 40735cd Compare December 23, 2023 10:28
@AndrewChubatiuk
Copy link
Author

@whynowy could you please review it again?

@AndrewChubatiuk AndrewChubatiuk force-pushed the replace-govaluate-with-expr branch 2 times, most recently from fddf83f to 47eccb2 Compare April 2, 2024 20:18
Signed-off-by: AndrewChubatiuk <andrew.chubatiuk@gmail.com>
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.

None yet

2 participants