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

Fix new graph parser bug (PR to 6.11.1) #2002

Merged
merged 4 commits into from
Sep 19, 2016

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Sep 17, 2016

First PR targeting the new 6.11.x branch.

Conditional expression processing is failing if the same task name appears in an expression with and without a cycle point offset, e.g.:

graph = foo | foo[-P1D] => bar

(NIWA has an operational suite with this sort of construct in it).

cylc val test
2016-09-17T23:23:01Z ERROR - name 'foo_colon_succeed_leftsquarebracket__minus_P1D_rightsquarebracket_' is not defined
'"(foo_colon_succeed | foo_colon_succeed_leftsquarebracket__minus_P1D_rightsquarebracket_)"'
'ERROR, bar: invalid trigger expression.'

This shows the problem is that the explicit :succeed trigger is getting substituted in before the offset foo:succeed[-P1D] instead of after it foo[-P1D]:succeed. (only if the same name occurs with and without an offset in the same expression).

This PR branch includes two commits merged to master since 6.11.0 as well: #1999 and #2000. Both affect only the test battery.

I'll make a copy of the branch for a PR to master as well.

matthewrmshin and others added 3 commits September 17, 2016 22:05
Ensure that it overrides a `True` setting in `global.rc`.
Use common test infrastructure to set up temporary suite location, etc.
For conditional expressions containing same task name with and without
an offset, e.g. "foo | foo[-P1D] => bar".
@hjoliver hjoliver added the bug label Sep 17, 2016
@hjoliver
Copy link
Member Author

@matthewrmshin - please review and/or assign

@hjoliver hjoliver added this to the 6.11.1 milestone Sep 17, 2016
@hjoliver hjoliver changed the title Fix new graph parser bug Fix new graph parser bug (PR to 6.11.1) Sep 17, 2016
@@ -317,10 +317,13 @@ def _proc_dep_pair(self, left, right):
n_info = []
expr = left
for name, offset, trig in info:
offset = offset or ''
Copy link
Member Author

Choose a reason for hiding this comment

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

No offset already results in an empty string here, not None.

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.

Looks OK to me. Test battery OK in my environment.

@matthewrmshin
Copy link
Contributor

@arjclark please do a quick sanity check.

@arjclark
Copy link
Contributor

@hjoliver - could you note what those two commits are that you're bundling in with this in the opening post for ease of tracking what's gone into 6.11.x at a later date.

@matthewrmshin
Copy link
Contributor

#1999 and #2000. Both affect only the test battery.

@arjclark
Copy link
Contributor

Looks OK to me. No problems from the test-battery in my environment.

@arjclark arjclark merged commit b7d0b33 into cylc:6.11.x Sep 19, 2016
@hjoliver hjoliver deleted the fix.graph-parser-bug branch September 19, 2016 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants