-
Notifications
You must be signed in to change notification settings - Fork 59
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
Implement #250 #325
base: master
Are you sure you want to change the base?
Implement #250 #325
Conversation
The tests seem to fail because of:
Funnily enough that test sounds like the exact opposite of what we want with #250 so it should be correct the test is failing on this. Particulary this: # Even with a family, if an instance is not requested
# it still processes.
class ValidateContext(pyblish.api.Validator):
families = ["NOT_EXIST"]
def process(self, context):
count["#"] += 1 |
Was this ever solved with another PR - or @tokejepsen I remember you mentioned you were having problems with the ContextPlugins always being picked up before too - which this PR would've solved. Any reason this is still open without any comments? :) The reason could be this comment? |
Think I've solved my issue with ContextPlugins by doing this family check early in the plugin. Would be nice with this feature :) |
@mottosso any pointers on how you feel it's best to proceed with this? we're still hitting this issue here and there. |
Think the biggest issue will be backward compatibility on this. Maybe it needs to be tackle with an opting-in environment variable, similar to #332. |
Yes, an opt-in would be preferable. The way forwards could be to keep adding opt-ins, till we eventually find equilibrium with a few opt-ins being permanently opted in at which point it's time for a 2.0 (where it's OK to break backwards compatibility). For this feature, the important thing is that it works the same in Lite and QML as well (which may already happen as a result of implement it for base). |
Thanks for confirming. I hope to find some time to take a look this week and get this fixed for all. 👍 |
Let's collect opt-in environment variables like this. Then I've got some ideas for a better way of documenting the API. |
Woop, there it is. @mottosso @tokejepsen You guys probably know about the following: Remember me again why Why couldn't it just use But wouldn't (shouldn't?) the GUI do the same? Because it would just "hold" the rest of the iterator after collection until one decides to continue to process. In the meantime the user could change possible states (like active of the plug-ins)... thus the iterator would behave as expected. |
It's because the GUI needs to know which plugin/instance pair runs, before the iterator has returned it. The iterator only returns the result of an operation. It should be more unified, I just never managed to get it working. |
Checking on the pulse of this issue. Would absolutely be great to have contextPlugins respect families. |
This PR should probably be updated to respect https://api.pyblish.com/pages/PYBLISH_EARLY_ADOPTER.html |
Yes! It's very possible to implement this now; either as an early-adopter feature (i.e. asserting that it will be the default of 2.0, still to be discussed) or as an opt-in feature (possibly also available but not default in 2.0). |
I think the deprecation system in Maya 2018 can be a good fit for us. In a nutshell, it has a flag to disable all deprecated functionality, throwing errors when they're used. Basically the opposite of an opt-in. In our case, it'd be "services" and old "CollectorPlugin" etc. And, if this were to be a winner, the old behavior of contexts not respecting families. |
Thank you for the prompt responses. Glad to see there is support for this in the works. It looks like it is waiting in a pr? |
This is a PR. :) You can give Roy's fork a try, see if it achieves what you're looking for. |
This implements #250 where ContextPlugins that filter to specific families (so
"*" not in plugin.families
) will not run when no instance present that the given family.When the ContextPlugin filters to two families and both are present by instances the ContextPlugin will still only run once.