fix BETWEEN operator; ensure task cancellation when terminating workflow #1734
Conversation
apanicker-nflx
requested review from
kchristidis,
AlejandroJorgePerez and
aravindanr
Codecov Report
@@ Coverage Diff @@
## dev #1734 +/- ##
============================================
- Coverage 64.97% 64.87% -0.11%
- Complexity 3757 3781 +24
============================================
Files 289 289
Lines 17856 17932 +76
Branches 1614 1620 +6
============================================
+ Hits 11602 11633 +31
- Misses 5441 5478 +37
- Partials 813 821 +8 Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build 4230
💛 - Coveralls |
apanicker-nflx
force-pushed
the
minor_fixes
branch
2 times, most recently
from
be7b864
to
22a7821
Compare
| @@ -70,7 +62,7 @@ protected void _parse() throws Exception { | |||
| this.value = "IS"; | |||
| }else if(peeked[0] == '!' && peeked[1] == '='){ | |||
| this.value = "!="; | |||
| }else if(peeked.length == betwnLen && new String(peeked).equals(Operators.BETWEEN.value())) { | |||
| }else if(peeked[0] == 'B' && peeked[1] == 'E' && peeked[2] == 'T' && peeked[3] == 'W' && peeked[4] == 'E' && peeked[5] == 'E' && peeked[6] == 'N'){ | |||
There was a problem hiding this comment.
Couple of things:
- Now that we're not checking length of peeked, could it possibly throw ArrayIndexOutOfBounds exception?
- Given that
STARTS_WITHbelow still checks with String equals, what was the case for checking each character by element?
There was a problem hiding this comment.
Ahh good catch on the ArrayIndexOutOfBounds exception. Pushed out a fix for this.
There was a problem hiding this comment.
The parse method always peeks the maxOperatorLength number of characters. After the STARTS_WITH operator was added, the length of this operator is now the max length (11). Now consider the search query excerpt startTime BETWEEN 100 AND 200. In this case, the parse method would now parse the following string BETWEEN 100 which is of length 11. This would fail the String equals comparison and the BETWEEN operator is now non-functional.
apanicker-nflx
force-pushed
the
minor_fixes
branch
from
22a7821
to
c627e2f
Compare
apanicker-nflx
force-pushed
the
minor_fixes
branch
from
c627e2f
to
9df9e2b
Compare
Fix
BETWEENoperatorFix the parsing of the operator
Ensure task is cancelled when terminating a workflow
Remove tasks from the queue first so that polling for tasks will not retrieve these
Update task status to CANCELED
Update workflow status to TERMINATED