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

cylc gscan: add --name option #2065

Merged
merged 4 commits into from
Nov 16, 2016
Merged

Conversation

arjclark
Copy link
Contributor

Partially addresses #1625

Adds --name option for filtering suites to display in gscan as per cylc scan.

@oliver-sanders - please review1
@matthewrmshin - please review2

@arjclark arjclark added this to the next release milestone Nov 15, 2016
@matthewrmshin
Copy link
Contributor

Looks like PEP8 test failing on Travis CI.

@arjclark
Copy link
Contributor Author

Pep8 fixed.

Copy link
Contributor

@matthewrmshin matthewrmshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment. OK otherwise.

name_pattern = name
else:
name_pattern = ['.*']
name_pattern = "(" + ")|(".join(name_pattern) + ")"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to compile the pattern here with re.compile - to catch any re.error that it may generate. (In addition, using a compiled regular expression object's .match method for matching a string should in theory be faster than using re.match.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now being compiled.

@@ -1126,6 +1144,9 @@ def update(self):
self.tasks_by_state[(suite, host)] = suite_info[
'tasks-by-state']

if not (re.match(self.name_pattern, suite)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should now be able to do self.name_pattern.match(suite).

Copy link
Contributor

@matthewrmshin matthewrmshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now good.

@matthewrmshin matthewrmshin removed their assignment Nov 16, 2016
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK, tested as working.

@oliver-sanders oliver-sanders merged commit cf6e75e into cylc:master Nov 16, 2016
@arjclark arjclark deleted the gscan.name_filter branch November 17, 2016 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants