DB workflow_params table: store None instead of deleting/not storing params

[MetRonnie]

Closes #5593

Discovered in #5592 where pause_workflow and resume_workflow were happening in the same operation (reload) so were getting committed in the same transaction. The workflow ended up paused in the DB, but unpaused in the workflow state.

So instead of deleting or not storing params, store them as None (which is an empty string for the TEXT type in sqlite). In the case of is_paused store it as a boolean ('0' or '1').

Full compatibility with pre-8.2.0 workflow databases is maintained. However, 8.2.0 workflow databases will not be compatible with earlier versions of Cylc.

Check List

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• No dependency changes
• Tests are included
• CHANGES.md entry included if this is a change that can affect users
• No docs change needed

[oliver-sanders]

👍

[oliver-sanders]

FYI, this is a negative change.

The way it was before, the string formatting only happened if the message was actually logged. With this change the string formatting happens, even if the message is not needed.

This is why the variables are provided as arguments to the logging function rather than '+ workflow UUID = %s' % value.

% formatting is still the documented advice.

https://docs.python.org/3/howto/logging.html#optimization

[MetRonnie]

Interesting to know 👍 However this method is only called once per restart and reload so I don't think it is significant in this case. Also most of these are INFO level so will be logged in the vast majority of cases

[oliver-sanders]

I'm not asking for changes, but please leave these out of future tidying (there's no gain to converting them). Profiling is showing logging to be a surprisingly large CPU sink.

For a more modern syntax I think positional {} formatting should work the same e.g:

LOG.debug('{0} something', itask)

[MetRonnie]

Ah, note if you try to restart a workflow ran with this version of Cylc in an earlier version, you're going to get

CRITICAL - Workflow shutting down - int() argument must be a string, a bytes-like object or a real number, not 'NoneType'

plus traceback. This is unavoidable

[oliver-sanders]

This is ok for a minor release.

We aim to keep the DB forward compatible (with upgraders) within reason, but backward compatibility is not a concern, cylc play makes it hard to do this and marks the option as NOT RECOMMENDED.