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

Review cylc-get-host-metrics function extract_pattern #3398

Closed
kinow opened this issue Oct 2, 2019 · 1 comment
Closed

Review cylc-get-host-metrics function extract_pattern #3398

kinow opened this issue Oct 2, 2019 · 1 comment
Assignees
Labels
bug Something is wrong :( small
Milestone

Comments

@kinow
Copy link
Member

kinow commented Oct 2, 2019

Describe the bug

Had a look at cylc-get-host-metrics today and noticed a warning in the IDE for the function extract_pattern. Copied below:

def extract_pattern(data_pattern, raw_string):
    """Try to return first parenthesized subgroup of a regex string search."""
    error_msg = "No 're.search' matches for:\n  %s\non the string:\n  %s" % (
        data_pattern.pattern, raw_string)
    try:
        matches = re.search(data_pattern, raw_string)
    except re.error:
        sys.stderr.write(error_msg)
    if matches:
        return matches.groups()
    else:
        raise AttributeError(error_msg)

The issue is that if we reach except re.error, and matches is not set, the next if matches statement would raise a NameError.

However; there is no way for re.error to happen I think. Because a few lines before we have data_pattern.pattern, which means that the data_pattern is not a string, but a compiled regex pattern.

Can't tell whether we should fix and accept string, or ignore the re.error, or just declare match somewhere else... a bit confusing.

Low priority issue, probably will go away with future changes.

Release version(s) and/or repository branch(es) affected?

master and 8.0a1

Steps to reproduce the bug

NA, couldn't write a unit test failing as well, because I think re.error cannot be raised 😕

Expected behavior

I guess type hints either in the comments or signature to indicate what the data_pattern type is, and simplify the code.

Screenshots

Additional context

Pull requests welcome!
This is an Open Source project - please consider contributing a bug fix
yourself (please read CONTRIBUTING.md before starting any work though).

@kinow kinow added the bug Something is wrong :( label Oct 2, 2019
@kinow kinow added this to the some-day milestone Oct 2, 2019
@oliver-sanders oliver-sanders mentioned this issue Jan 28, 2020
9 tasks
@kinow kinow self-assigned this Mar 15, 2020
@oliver-sanders
Copy link
Member

Closed by #3489

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :( small
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants