pyblish / pyblish-base

Pyblish base library - see https://github.com/pyblish/pyblish for details.
Other
123 stars 60 forks source link

Fix published signal being emitted from collect, validate, extract or integrate #360

Closed Jimmy-LeeBoisvert closed 4 years ago

Jimmy-LeeBoisvert commented 4 years ago

Description

The "published" signal is currently being emitted when we call the collect, validate, extract and integrate functions, despite these functions already emitting their own signal (e.g. "collected", "validated", etc.) This is because these functions all end up calling the _publishiter function, which emits the "published" signal.

I'm not sure if this is the intended behavior, but it seems like a bug to me.

The publish function is the only one that can be used to execute the plugins regardless of their phase, and it seems to make sense to emit the "published" signal when using it, as the name of the signal seems related to the function's name. However, when the collect, validate, extract or integrate functions are used, it seems implied that we'd deliberately want to execute a single specific phase and would thereby expect a different signal.

Expected results

It appears that the "published" signal is meant to be emitted when the whole publish process is done, but when we call the collect, validate and extract functions individually (and to a certain extent the integrate function), the publish process is incomplete, and therefore the "published" signal should not be emitted.

In fact, this seems to be the behavior of the Pyblish UIs (QML and Lite), but these UIs don't appear to use the publish or _publishiter functions, as they instead appear to have their own implementations of how to execute plugins and emit signals.

Short reproducible

Imagine creating a small API where we want to split the collect, validate and extract/integrate phases like so:

import pyblish.api
import pyblish.lib
import pyblish.util

class Publisher(object):

    def __init__(self):
        self._context = None
        self._collected = False
        self._validated = False

    def collect(self):
        self._context = pyblish.util.collect()
        self._collected = True
        self._validated = False

    def validate(self):
        if not self._collected:
            self.collect()

        pyblish.util.validate(self._context)
        self._validated = True

    def publish(self):
        if not self._validated:
            self.validate()

        plugins = [
            plugin for plugin in pyblish.api.discover()
            if not pyblish.lib.inrange(number=plugin.order, base=pyblish.api.CollectorOrder)
            and not pyblish.lib.inrange(number=plugin.order, base=pyblish.api.ValidatorOrder)
        ]

        pyblish.util.publish(self._context, plugins)

class Collector(pyblish.api.ContextPlugin):
    order = pyblish.api.CollectorOrder

    def process(self, context):
        pass

class Validator(pyblish.api.ContextPlugin):
    order = pyblish.api.ValidatorOrder

    def process(self, context):
        pass

class Extractor(pyblish.api.ContextPlugin):
    order = pyblish.api.ExtractorOrder

    def process(self, context):
        pass

class Integrator(pyblish.api.ContextPlugin):
    order = pyblish.api.IntegratorOrder

    def process(self, context):
        pass

def callMeWhenPublishedIsOver(context):
    print("### GOOD NEWS! The publish is over!")

pyblish.api.deregister_all_paths()
pyblish.api.deregister_all_plugins()
pyblish.api.deregister_all_callbacks()

pyblish.api.register_callback("published", callMeWhenPublishedIsOver)
pyblish.api.register_plugin(Collector)
pyblish.api.register_plugin(Validator)
pyblish.api.register_plugin(Extractor)
pyblish.api.register_plugin(Integrator)

publisher = Publisher()
publisher.collect()
publisher.validate()
publisher.publish()

By executing this code, we currently end up calling the callMeWhenPublishedIsOver function 3 times, instead of the expected 1 time, but if we only register the callback and then use either the Pyblish-QML or Pyblish-Lite UIs, the callback function will only be called once, which is expected.

mottosso commented 4 years ago

Thanks for sharing this @jlboisvert007, I think you're right.

But I'm not sure about the added _publish_iter. That means we've got..

At first glance, it looks like maybe we can move the contents of _publish_iter into _convenience_iter, and have publish_iter use that as well, like the validate_iter and collect_iter etc does?

Jimmy-LeeBoisvert commented 4 years ago

Thanks for the quick reply @mottosso !

Agreed on moving the contents of _publish_iter into _convenience_iter. That should work too!

Jimmy-LeeBoisvert commented 4 years ago

Thanks @mottosso !