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

Evaluate a condition branch only if needed #51

Closed

Conversation

sewi-cpan
Copy link

@sewi-cpan sewi-cpan commented Dec 6, 2023

The CEL language definition suggests:

To get traditional left-to-right short-circuiting evaluation of logical operators, as in C or other languages (also called "McCarthy Evaluation"), the expression e1 && e2 can be rewritten e1 ? e2 : false. Similarly, e1 || e2 can be rewritten e1 ? true : e2.

cel-python used to evaluate the full tree before deciding if a branch will be used or not. This PR only evaluates a branch if selected by the condition.

Fixes #50

[The CEL language definition](https://github.com/google/cel-spec/blob/master/doc/langdef.md#logical-operators) suggests:
> To get traditional left-to-right short-circuiting evaluation of logical operators, as in C or other languages (also called "McCarthy Evaluation"), the expression e1 && e2 can be rewritten `e1 ? e2 : false`. Similarly, `e1 || e2` can be rewritten `e1 ? true : e2`.

`cel-python` used to evaluate the full tree before deciding if a branch will be used or not. This PR only evaluates a branch if selected by the condition.

Fixes issue cloud-custodian#50
Copy link

linux-foundation-easycla bot commented Dec 6, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: kapilt / name: Kapil Thangavelu (9c75fd3)
  • ✅ login: sewi-cpan / name: Sebastian Willing (1d6ce96)

@kapilt
Copy link
Collaborator

kapilt commented Dec 22, 2023

thanks for the pr, lgtm although a test would be appreciated, would also appreciate a look by @slott56

@slott56
Copy link
Collaborator

slott56 commented May 25, 2024

See #63 which fixes this problem.

@slott56 slott56 closed this May 25, 2024
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.

Condition syntax sometimes evaluates the wrong branch
3 participants