Fix missing "failed" actions on plugins with warnings instead of errors

[jboisvertrodeofx]

When plugins have warnings instead of errors, they are not considered for the available "failed" actions.

I thought there'd be 2 ways to handle this case:

1. Assume that plugins with either warnings or error have failed, and therefore use the same on action type: "failed".
2. Create a new on action type, "warning", in pyblish_base, and add support for warnings the core pyblish_base API instead of just pyblish-lite.

The second option seemed like it would be a lot more work, as warnings seem to currently only be supported in pyblish-lite itself, so I worked on the easier first option. Besides, I thought it made sense that warnings and errors would use the same action to be fixed, as the plugin identifies an issue and that issue is either critical or not, but the way to fix it should be with the same action in both cases.

Let me know what you think @mottosso !

[mottosso]

Nice work @jboisvertrodeofx.

The second option seemed like it would be a lot more work, as warnings seem to currently only be supported in pyblish-lite itself, so I worked on the easier first option.

That's true, but on the other hand it is a UI feature and only available in Lite and QML. I think it's fair to implement this here, rather than update pyblish-base.

With that in mind, maybe let's just add the action type "warning" here in Lite? Then it could be added to QML in the same manner (whenever somebody asks for/needs it). The only thing then missing from Base would be an updated docstring.

[jboisvertrodeofx]

Hello @mottosso ,

From pyblish-base the docstring would have to be updated, but also the MetaAction meta class, which checks if the value of on is supported, so that would have to be updated if we want to add warning as a new valid on value used by pyblish-lite (and eventually pyblish-qml): https://github.com/pyblish/pyblish-base/blob/3290fd5b51b496d431cc2ca608639e21e727ccbd/pyblish/plugin.py#L363

If we decide to add warning, it might make sense to start supporting lists/tuples for the on value, so that the same action could be used for both errors and warnings. However, I'm not sure how elegant that would be, considering that there is an all value that we wouldn't want to end up in a list with other values. For this reason, I kind of liked have the same failed value for both.

An alternative to supporting lists/tuples would be to keep failed for both errors and warnings, but also have error and warning for cases where we would want to use an action only for either an error or a warning.

[mottosso]

which checks if the value of on is supported

Oooh, I see. That's both good that it's checked, and too bad that it's checked. xD

it might make sense to start supporting lists/tuples for the on value

We could probably do "warningOrError" or a more general "problem" string.

An alternative to supporting lists/tuples would be to keep failed for both errors and warning

Ah, yes exactly. Except not "failed" because publishing won't stop on warnings so it wouldn't be a failure per se.

That leaves option 1, which is problematic because it changes the outcome of publishing. For those that trusted "warning" to not provide error actions that won't be the case anymore. And also it would be different across QML and Lite, which is bound to raise another issue here on GitHub in a flash.

I'd rather we bite the bullet, and add "warning" to base (I can merge that quick, as it's a backwards compatible update).

[jboisvertrodeofx]

I added new "warning" and "failedOrWarning" action types to pyblish-base and updated this PR so that "failed" would keep the existing behavior, "warning" would handle only plugins with warnings and "failedOrWarning" would handle both cases so that a single action can be used for plugins that have a warning or an error.

The pyblish-base PR is here: pyblish/pyblish-base#370

I made the same changes in pyblish-qml: pyblish/pyblish-qml#367

Let me know if that's fine by you!

[mottosso]

This looks good to me, I've merged and released the others, merging and releasing this now. :) Nice work @jboisvertrodeofx