-
Notifications
You must be signed in to change notification settings - Fork 329
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[OPIK-486] Add NOT_EQUAL operator for filtering #816
[OPIK-486] Add NOT_EQUAL operator for filtering #816
Conversation
2a2ef56
to
2e654ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A suggestion for tests, but not a blocker. You can add it in a follow-up PR.
@@ -12,6 +12,7 @@ public enum Operator { | |||
STARTS_WITH("starts_with"), | |||
ENDS_WITH("ends_with"), | |||
EQUAL("="), | |||
NOT_EQUAL("!="), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just make sure FrontEnd is happy with this syntax !=
for the query param. From my side, it looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FE confirmed that they are good: https://comet-ml.slack.com/archives/C0768KH9MDJ/p1733391186958639
@ParameterizedTest | ||
@MethodSource("equalAndNotEqualFilters") | ||
void getByProjectName__whenFilterTotalEstimatedCostEqual_NotEqual__thenReturnSpansFiltered(Operator operator, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's similar tests for equals in traces object (and maybe some other). It'd be nice to add a test for all where this applies, not only spans.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add in a later PR, we also have filters in Traces and Datasets.
Details
Add NOT_EQUAL operator for filtering
Resolves: #551
Testing
Added corresponding integration tests