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

Inheritance parsing - ignoring items if not all quoted #2700

Closed
arjclark opened this issue Jun 21, 2018 · 5 comments · Fixed by #3199
Closed

Inheritance parsing - ignoring items if not all quoted #2700

arjclark opened this issue Jun 21, 2018 · 5 comments · Fixed by #3199
Labels
bug Something is wrong :(
Milestone

Comments

@arjclark
Copy link
Contributor

Bit of a weird one - only hit it because of working with parameterization.

Have written task inheritance in two ways. In the first, the non-quoted item doesn't get picked up:

inherit = 'MAINFAM<major, minor>', SOMEFAM

Whereas when each item is quoted in the list the do all get picked up:

inherit = 'MAINFAM<major, minor>', 'SOMEFAM'

Had to put MAINFAM, in quotes because of the need to (according to validation) with two items to iterate over - presumably to identify it as a single item. You can replicate this behavior with non-parameter based inheritance though (although it would be odd to quote an item if you didn't need to for parameterization):

inherit = 'BIGFAM', SOMEFAM

Depending on the rest of your suite graphing and runtime you may or may not miss that the SOMEFAM type entry gets missed.

@arjclark arjclark changed the title Inheritance parsing - ignoring items Inheritance parsing - ignoring items if not all quoted Jun 21, 2018
@matthewrmshin
Copy link
Contributor

Does it work if you don't quote anything?

@matthewrmshin matthewrmshin added this to the soon milestone Jun 21, 2018
@matthewrmshin matthewrmshin added the bug? Not sure if this is a bug or not label Jun 21, 2018
@arjclark
Copy link
Contributor Author

You can't actually get that past validation as that errors with:

[FAIL] ERROR: names containing commas must be quoted (e.g. 'foo<m,n>')

@arjclark
Copy link
Contributor Author

arjclark commented Jun 21, 2018

Potentially anticipating your next question, you also can't do (which is closer to my real use-case rather than the simplified one above):

'MAINFAM<major, minor>, SOMEFAM'

as it only expands out the first MAINFAM item but not the SOMEFAM one (still reads it as SOMEFAM<minor> and errors validation as: [FAIL] 'ERROR, undefined parent for coupled_m1_s01: MAINFAM_m1_s01, SOMEFAM<mem>')

@matthewrmshin matthewrmshin added bug Something is wrong :( and removed bug? Not sure if this is a bug or not labels Jun 21, 2018
@arjclark
Copy link
Contributor Author

arjclark commented Jun 21, 2018

Further experimenting also shows an issue with: inherit=MAINFAM_M<major>_S<minor>, as this also fails to expand out the second item as:

ERROR, undefined parent for ... : MAINFAM_M1_S<step>

albeit in the same entry now!

@matthewrmshin - would you like me to capture that in a separate issue or just leave it in here as the two are (presumably) inter-related?

@kinow
Copy link
Member

kinow commented Nov 12, 2018

The problem is in the validate.py, the regex used for 'BIGFAM', SOMEFAM is '([^'\\]*(?:\\.[^'\\]*)*)'.

Follow Pythex link to see that the match finds BIGFAM and then just gives up on SOMEFAM.

The naive solution, I think, would be to just add more to that regex and increase its complexity, but thus allowing it to find matches of single-quoted, plus non-quoted values.

But then the issue would be whether mixing quotes would be OK too. i.e., is this valid?

inherit = "BIGFAM", 'SOMEFAM', FAM

Unit test to reproduce the issue:

import unittest
import tempfile

from cylc.config import SuiteConfig


class TestConfig(unittest.TestCase):

    def test_inheritance(self):
        template_vars = {}
        with tempfile.NamedTemporaryFile() as tf:
            tf.write("""
[scheduling] 
    [[dependencies]] 
        graph = '''hello => BYE
                   hello => CIAO
        ''' 
[runtime] 
    [[hello]] 
    [[BYE]] 
    [[CIAO]] 
    [[ goodbye_0 ]] 
        inherit = 'BYE', CIAO
            """)
            tf.flush()
            config = SuiteConfig('test', tf.name, template_vars=template_vars)
            self.assertTrue('goodbye_0'
                            in config.runtime['descendants']['BYE'])
            self.assertTrue('goodbye_0'
                            in config.runtime['descendants']['CIAO'])

It will fail in the last assertTrue, due to CIAO being missing in the list of nodes with descendants.

kinow pushed a commit to kinow/cylc-flow that referenced this issue Jun 25, 2019
@matthewrmshin matthewrmshin modified the milestones: soon, cylc-8.0a1 Jun 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants