[rayci] support select & tag together as a filter #234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: can <[email protected]>
aslonnie
reviewed
May 9, 2024
raycicmd/step_filter.go
Outdated
Comment on lines
63
to
75
| func newSelectAndTagsStepFilter( | ||
| skipTags []string, selects []string, filterCmd []string, | ||
| ) (*stepFilter, error) { | ||
| select_filter := newSelectStepFilter(skipTags, selects) | ||
| tag_filter, err := newTagsStepFilter(skipTags, filterCmd) | ||
|
|
||
| return &stepFilter{ | ||
| skipTags: skipTags, | ||
| selects: select_filter.selects, | ||
| runAll: tag_filter.runAll, | ||
| tags: tag_filter.tags, | ||
| }, err | ||
| } |
There was a problem hiding this comment.
so, maybe just do this:
- have a
newStepFilterthat takes selects and tags - rename
newTagsStepFilterintostepFilterFromCmd, and when it gets the tags, callnewStepFilter()?
raycicmd/step_filter.go
Outdated
| @@ -22,12 +22,29 @@ func (f *stepFilter) reject(step *stepNode) bool { | |||
| } | |||
|
|
|||
| func (f *stepFilter) accept(step *stepNode) bool { | |||
| if f.selects != nil && f.tags != nil { | |||
| // in both key selection mode, hit when the step has both keys and tags | |||
| return f.acceptSelectHit(step) && f.acceptTagHit(step) | |||
There was a problem hiding this comment.
I feel that we can make this function always return:
f.acceptSelectHit(step) && f.acceptTagHit(step)
and treat missing selects always as return true?
probably needs to change the runAll field in the stepFilter into runAllTags now, as it semantically only applies to tags selection.
Signed-off-by: can <[email protected]>
can-anyscale
force-pushed
the
can-select-and-tag-filter
branch
from
dec277f to
ac6c079
Compare
|
@aslonnie's comments, combining both tag and select filter |
aslonnie
approved these changes
May 9, 2024
|
let's go! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If both selects and tags are provided as a filter, select nodes based on both filtering. I need this for using select in microcheck, otherwise we will incur costs by saving all microcheck tests on PR. Let me know if there is a better alternative.
Test: