cylc-flow icon indicating copy to clipboard operation
cylc-flow copied to clipboard

Better validation deprecation warning format?

Open hjoliver opened this issue 2 years ago • 7 comments

I have a user at NIWA who really dislikes the current formatting of config item deprecation warnings.

Example:

$ cylc val .
WARNING - deprecated items were automatically upgraded in "workflow definition"
WARNING -  * (8.0.0) [runtime][clean][job][execution retry delays] -> [runtime][clean][execution retry
    delays] - value unchanged
Valid for cylc-8.1.0.dev

Contraction of multiple square brackets to convey nested headings could be confusing

  • for this, I don't think use of all the brackets on one line is viable as it makes a mess, so I've added a doc link instead: https://github.com/cylc/cylc-doc/pull/532

In some cases as above (but not all cases) we are enclosing the final item name in brackets too, which wrongly suggests another heading

  • I think this is a bug, should be fixed

Question: should we also have a verbose mode output that does not contract items to a single line, to avoid any possible confusion?

For the above example:

WARNING (8.0.0) - deprecated item:
    [runtime]
        [[clean]]
            [[[job]]]
                execution retry delays = PT10S
Upgraded to:
    [runtime] 
        [[clean]]
             execution retry delays = PT10S

hjoliver avatar Sep 05 '22 02:09 hjoliver

The full format is too long for documentation, links and warnings and the like which is why the shorthand exists. The shorthand is introduced and explained in the tutorial. The shorthand is used throughout the documentation and every documented configuration has it's shorthand (path) listed in the reference documentation.

I would prefer to improve the visibility of the shorthand to people who haven't gone through the tutorials (so as to make the documentation & defaults more intuitive) than to add an extra command line option.

oliver-sanders avatar Sep 05 '22 09:09 oliver-sanders

It would be good to turn the shorthand configurations we log into URLs that point at the reference documentation online.

Perfectly possible (control characters handle linking from the terminal), it's just tricky pointing the links at the right version of the docs, testing and working out how to handle link breakages - https://github.com/cylc/cylc-flow/issues/4663

oliver-sanders avatar Sep 05 '22 09:09 oliver-sanders

The full format is too long for documentation, links and warnings and the like which is why the shorthand exists.

Agreed, I'm not suggesting changing that.

The shorthand is introduced and explained in the tutorial.

Yes, but existing users upgrading existing workflows to Cylc 8 are unlikely to work through the tutorial.

The shorthand is used throughout the documentation and every documented configuration has it's shorthand (path) listed in the reference documentation.

Yep, that's fine. And my cyc-doc PR, referenced above, links from the migration guide to the shorthand documentation.

than to add an extra command line option.

Not suggesting that. By "verbose mode validation" I just meant the existing -v validation option.

  • cylc validate - print shorthand deprecation upgrade warnings, which (evidently!) may require some thought to interpret
  • cylc validate -v - print long-format deprecation upgrading warnings, which really can't be misinterpreted

hjoliver avatar Sep 05 '22 10:09 hjoliver

So what do you think of printing validation deprecation warnings in long format, as in the example in the description, at least with the '-v' option?

Again, this is motivated by a user struggling to understand the shorthand format during Cylc 8 upgrade. And on considering his complaint, I think it is somewhat valid ... although part of the problem was (a) the formatting bug now fixed in #5119; and (b) no link to the shorthand format documentation in the migration guide, fixed in cylc/cylc-doc#532.

BTW I'm not completely sold on this, but on reflection I do think the current formatting isn't great in the terminal (it's also invariably too wide, so it wraps around messily), and is there any good reason not to be more verbose for clarity reasons? These are warnings that we want users to address, after all.

At the minimum, we should print a reference to the shorthand documentation at the top of validation stdout.

hjoliver avatar Sep 06 '22 01:09 hjoliver

The short format is pretty fundamental, it's the very first thing introduced in the tutorial and is used throughout the documentation and cylc-flow warnings/errors.

Screenshot from 2022-09-06 09-52-44

Other languages have different syntax for constructing and representing objects E.G:

Python:

# module foo
class Bar:
    def baz():

# foo.Bar.baz

JSON schema:

{
   'foo': {
       'bar': {
           'baz': {}
        }
    }
}
# x/foo/bar/baz

Etc.

The shorthand is a fundamental parsec feature which anyone who works with Cylc config files is going to have to learn. I don't think that outputting the long format in verbose mode is really going to help, if it's really incomprehensible then so are the docs?!

I think we should:

oliver-sanders avatar Sep 06 '22 08:09 oliver-sanders

Longer term it might be a good idea to change the formatting of validation output to be more config orientated e.g:

<level> <config>
    * message
    * ...
WARN [runtime][...]
    * Parent not defined [...]
INFO [scheduler][...]
    * Such and such.

This would scan better but would also be more ingestible for text editor plugins.

oliver-sanders avatar Sep 06 '22 08:09 oliver-sanders

OK, I'm happy with that :+1:

hjoliver avatar Sep 06 '22 10:09 hjoliver