David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/10922 )
Change subject: IMPALA-7279: Fix flakiness in test_rows_availability ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py File tests/util/parse_util.py: http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@76 PS1, Line 76: r'(?P<value>[0-9]+(\.[0-9]+)?)(?P<units>\D+)' There's something weird about your pattern. If I print out the result of applying re.findall() to your sample duration string, each match winds up being a tuple with 3 elements: [('1', '', 'h'), ('2', '', 'h'), ('3', '', 'm'), ('4', '', 's'), ('5.6', '.6', 'ms'), ('4.5', '.5', 'us'), ('7.8', '.8', 'ns')] ...when you *probably* want each tuple to have 2 elements -- just the value and the units, like this: [('1', 'h'), ('2', 'h'), ('3', 'm'), ('4', 's'), ('5.6', 'ms'), ('4.5', 'us'), ('7.8', 'ns')] I think this pattern will give you that (but I'm not a regex expert): r'(?P<value>[0-9]+\.?[0-9]*?)(?P<units>\D+)' Furthermore, you're using the semantics for named groups (i.e., ?P<value> and ?P<units>), but then resorting to referencing by index numbers. You could try something like this: pattern = r'(?P<value>[0-9]+\.?[0-9]*?)(?P<units>\D+)' matches = re.finditer(pattern, duration) for match in matches: parsed = match.groupdict() if parsed['units'] == 'h': hours = float(parsed['value']) etc... ...which is a little more readable than [0] and [2]. -- To view, visit http://gerrit.cloudera.org:8080/10922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be Gerrit-Change-Number: 10922 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-Comment-Date: Wed, 11 Jul 2018 19:18:24 +0000 Gerrit-HasComments: Yes
