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

basic cylc 8 syntax checker to assist upgrade #4519

Closed
oliver-sanders opened this issue Nov 16, 2021 · 3 comments · Fixed by #4900
Closed

basic cylc 8 syntax checker to assist upgrade #4519

oliver-sanders opened this issue Nov 16, 2021 · 3 comments · Fixed by #4900
Assignees
Labels
Milestone

Comments

@oliver-sanders
Copy link
Member

Cylc gives deprecation warnings which is helpful.

Unfortunately, because of Jinja2 it is likely that there are bits of the config that Cylc will not load:

#!Jinja2
[scheduling]
    [[dependencies]]
        [[[P1D]]]
            graph = ...
{% if COND %}
         [[[PT6H]]]
             graph = ...
{% endif %}

[runtime]
    [[x]]
        ...
{% include "site/" + SITE + '.rc' with context %}

With Cylc 7 we had a lot of cases where suites would break after users turned on an option that hadn't been used in a while.

To make life easier we could consider providing a simple script that picks out the most obvious bits of invalid syntax without parsing the file.

Example:

Here's a quick and simple approach using sed that adds comments after lines that contain Cylc 7 syntax.

# 728gen.py
UPG = {                                                              
    '[[[remote]]]': 'flow.cylc[runtime][<namespace>]platform',      
    'graph.*=': 'flow.cylc[scheduling][graph]<recurrence>',    
}                                                    
                                                     
REGEX_CHARS = ['[', ']', '(', ')']                   
                                                     
                                                     
def escape(pattern):                                 
    for char in REGEX_CHARS:                         
        pattern = pattern.replace(char, rf'\{char}')    
    return pattern                                   
                                                     
                                                     
def main(write):                                     
    for pattern, msg in UPG.items():                 
        write(rf's|^\([^#]*{escape(pattern)}.*\)$|\1  # <= {msg}|')                  
                                                                     
                                                                     
if __name__ == '__main__':                                           
    main(write=print)  
$ python 728gen.py > 728.sed
$ sed -f 727.sed suite.rc
[scheduling]
    [[dependencies]]
        [[[R1]]]
            graph = """  # <= flow.cylc[scheduling][graph]<recurrence>
                foo
            """

[runtime]
    [[foo]]
        [[[remote]]]  # <= flow.cylc[runtime][<namespace>]platform
            host = foo

Pull requests welcome!

@oliver-sanders oliver-sanders added this to the cylc-8.0.0 milestone Nov 16, 2021
@hjoliver
Copy link
Member

Unfortunately, because of Jinja2 it is likely that there are bits of the config that Cylc will not load:

I was just about to add a warning to the 7-8 migration docs about this. I'll hold off in case we decide to go with this approach.

@hjoliver hjoliver added the question Flag this as a question for the next Cylc project meeting. label Nov 18, 2021
@wxtim
Copy link
Member

wxtim commented Jan 13, 2022

I was just about to add a warning to the 7-8 migration docs about this. I'll hold off in case we decide to go with this approach.

I think the warning in the documentation is valid either way.

@oliver-sanders
Copy link
Member Author

We think this would be a good idea, possibly fronted by a cylc command. Can be implemented via find -exec sed or manually via Python.

This becomes important when people start upgrading workflows out of Cylc 7 compatibility mode.

@oliver-sanders oliver-sanders removed the question Flag this as a question for the next Cylc project meeting. label Apr 7, 2022
@wxtim wxtim self-assigned this May 12, 2022
@wxtim wxtim mentioned this issue May 27, 2022
7 tasks
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 a pull request may close this issue.

3 participants