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

'publish' data member on instance doesn't work #59

Open
mkolar opened this Issue Aug 19, 2016 · 18 comments

Comments

Projects
None yet
5 participants
@mkolar
Copy link
Member

mkolar commented Aug 19, 2016

Problem

Setting instance.data['publish'] to False in the collector makes it be ignored by it's respective validators, even after it's turned back on in the GUI.

We noticed this when a lot of our plugins started failing using lite, but worked when using qml. Turns out it's because we use a lot of instances that are toggled off by default which prevents them from being validated properly, leading into many many failed publish attempts.

@mottosso mottosso added the bug label Aug 19, 2016

@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Aug 19, 2016

There's an update to pyblish-base that addresses this globally, you could start looking towards that. It hasn't been released yet, but it's in the latest commit of the master branch.

This issue will then be resolved as a side-effect of this getting into the next release.

@larsbijl

This comment has been minimized.

Copy link
Member

larsbijl commented Sep 29, 2016

it's is also affecting us. has 1.4.2 changed address this in base?

@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Sep 29, 2016

This should be fixed in pyblish-base 1.4.2

See here: https://github.com/pyblish/pyblish-base/blob/master/pyblish/logic.py#L304

Lite uses this iterator in control.py.

Let me know if this isn't working for you, and I'll take a closer look!

@larsbijl

This comment has been minimized.

Copy link
Member

larsbijl commented Sep 29, 2016

I'm using pyblish-base 1.4.2 and pyblish-lite 0.4.0 and i'm still having this issue

how is the instance publish update from pyblish-lite?

@larsbijl

This comment has been minimized.

Copy link
Member

larsbijl commented Sep 29, 2016

looks like the validators are skipped before the UI gets a chance to set the publish value to true.

https://github.com/pyblish/pyblish-base/blob/master/pyblish/logic.py#L305

WARNING:pyblish.logic:render_cache_et1 v007 was inactive, skipping..
render_cache_et1 v007 140232437144224 key publish value True
@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Sep 29, 2016

Can you tell whether this happens exclusively on the first instance in the list?

@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Sep 29, 2016

Ok, I've located the bug. This should only affect the first instance in the list.

It's happening, because of the generator which produces plug-in/instance pairs. In order for Lite to know when to stop, it has to prime the generator for the next pair before making a decision. It does this priming just before passing control to the GUI.

So in practice what happens is that the GUI has already determined the fate for the next item in the generator, and won't listen to changes in the GUI before moving on to process it.

  1. Processes current pair
  2. Fetches next pair
  3. Makes decision about whether or not to continue

It's a tricky one!

The pair must be fetched before being able to make a decision, but pairs cannot be put back into the generator if it decides not to continue.

pyblish-qml doesn't have this bug, because QML does not implement the "continue on pause/validation" that Lite has.

Suggestions are welcome.

@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Sep 29, 2016

And as a temporary workaround, add an additional null-instance first in the list, and publish=False by default.

@mottosso mottosso added the critical label Oct 9, 2016

@tokejepsen

This comment has been minimized.

Copy link
Member

tokejepsen commented Nov 16, 2016

Reproducible:

import pyblish_lite
import pyblish.api


class Collect(pyblish.api.ContextPlugin):

    order = pyblish.api.CollectorOrder

    def process(self, context):

        instance = context.create_instance(name="A")
        instance.data["publish"] = False
        context.create_instance(name="B")


class Validate(pyblish.api.InstancePlugin):

    order = pyblish.api.ValidatorOrder

    def process(self, instance):

        self.log.info(instance)

pyblish.api.register_plugin(Collect)
pyblish.api.register_plugin(Validate)

pyblish_lite.show()
@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Nov 16, 2016

Can you confirm that it only affects the first instance in the GUI? The following instances should work fine.

@tokejepsen

This comment has been minimized.

Copy link
Member

tokejepsen commented Nov 16, 2016

Yeah, it works if the first instance has instance.data["publish"] = True.

@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Nov 16, 2016

Is the problem visible on the other instances too? As in, can you set publish=False on the second instance, toggle it, and having it be processed?

On the first instance, if set to False, no amount of toggling would make it process.

@tokejepsen

This comment has been minimized.

Copy link
Member

tokejepsen commented Nov 16, 2016

Is the problem visible on the other instances too? As in, can you set publish=False on the second instance, toggle it, and having it be processed?

Yeah, I've tested with the workaround of having a dummy first instance that is publishable.

@darkvertex

This comment has been minimized.

Copy link
Contributor

darkvertex commented Apr 5, 2018

Hey all, experiencing this too and it's surprising the last update was from 2016 and it's not solved yet.

Here's some code to repro:

import pyblish.api
import pyblish_lite


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

    def process(self, context):
        for i in range(4):
            name = 'potato_{0}'.format(i)
            inst = context.create_instance(name, label=name, family='vegetable')
            inst.data['icon'] = 'male'
            inst.data['optional'] = True

            # Leave them all OFF except the last one:
            inst.data['publish'] = name == 'potato_3'


class FakeValidator(pyblish.api.InstancePlugin):
    order = pyblish.api.ValidatorOrder
    families = ['vegetable']

    def process(self, instance):
        raise pyblish.api.ValidationError('Boo!')

def setupPlugins():
    pyblish.api.deregister_all_plugins()
    pyblish.api.deregister_all_paths()
    pyblish.api.register_plugin(FakeCollector)
    pyblish.api.register_plugin(FakeValidator)


def startUI():
    setupPlugins()
    pyblish.api.register_host("python")
    window = pyblish_lite.show()

startUI()

The code above should give you something like this:
image
and if you try to Publish, fails as expected...
image


Now try to turn off the last one and toggle other ones:
image
and try to validate, you will get:
image

(Notice how it wrongly attempts to validate the last one, just because it had been ON at first.)


Let's try toggling ON just the 1st:

(If I try toggling all OFF it won't even let me press the button, which is ok I guess.)

image

and you will still get the last one being validated on as well as the first:

image


What is strange is that it almost does the right thing. It seems that the ones that were toggled ON initially will always be validated, but otherwise it will respect the toggling by the user.

I've been pushing for Pyblish at the studio where I work and this is a bit of a dealbreaker for me. :/ I would love to see this fixed.

@mottosso

This comment has been minimized.

Copy link
Member

mottosso commented Apr 6, 2018

It's true, it's been around for a little too long.

pyblish-lite is a "community version" of pyblish-qml, which on one hand means this is where you get the most freedom to change things around and implement issues (such as this) that suit you best. On the other it means there is less of a timeline to rely on and anyone to pressure into seeing anything fixed.

For pushing to a studio I would strongly suggest you choose pyblish-qml as it is the most mature and production proven.

To fix this issue, the problem is almost entirely found in this loop here. What you can do is fetch the next "current_pair" and immediately process it, as opposed to fetching it separately. Fetching it separately is what enables us to "look ahead" at which combination of plug-in/instance is going to be processes and it is because of this "look ahead" that the first instance doesn't respond to the toggle - it has already been "looked ahead" on start-up and is ready to go.

Let me know if that makes sense, would be happy to help seeing this fixed.

@darkvertex

This comment has been minimized.

Copy link
Contributor

darkvertex commented Apr 6, 2018

Thanks for pointing me to the loop you suspect is the culprit. I'm going to fire up a debugger and see if I can figure out a fix.

I also plan on exploring pyblish-qml now that I understand (as per your response in gitter chat) that it can actually work on earlier versions of Maya without Qt5 as the UI runs separately. This wasn't clear to me on first glance, but thanks for clarifying that.

darkvertex added a commit to darkvertex/pyblish-lite that referenced this issue Nov 1, 2018

control.py & window.py: Fix issue pyblish#59 with plugin and instance…
… states being out of date after initializing the pair_generator the first time.
@darkvertex

This comment has been minimized.

Copy link
Contributor

darkvertex commented Nov 1, 2018

It seems the problem is that self.pair_generator around here:

self.pair_generator = self._iterator(self.plugins,

is only created once after reset(), but inside the loop of the iterator the data can be out of date with the state of the UI's widgets.

Inside the _iterator()

def _iterator(self, plugins, context):

there is some filtering out of deactivated plugins and disabled instances, but diving in with a debugger shows us that the UI can go out of sync with what that loop thinks the .data of instances is (and I suspect a similar issue with optional plugins.) It's almost as if it took a snapshot when generating the iterator, then it's never reading the state "live".

The way it is all designed around iterating on a main iterator, I'm not entirely sure what the best approach would be to solve this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment