search

pyblish / pyblish-qml

Pyblish QML frontend for Maya 2013+, Houdini 11+, Nuke 8+ and more
GNU Lesser General Public License v3.0
115 stars 44 forks source link

Fix error handling within iterator #362

Open buddly27 opened 4 years ago

buddly27 commented 4 years ago

This resolves #353

Tested with PySide2 and Python 2.7, Python 3.7 and Python 3.8 with the demo window:

python -m pyblish_qml --demo --targets default

(took me a while to figure out that I had to pass the 'default' target to see something!)

I didn't deal with unit tests as I am unfamiliar with nose, please let me know if you want me to handle it.

buddly27 commented 4 years ago

I reverted to using the logic suggested in https://github.com/pyblish/pyblish-qml/issues/353#issuecomment-584159312.

Removing usage of StopIteration would require more thoughts to prevent the issues you highlighted.

mottosso commented 4 years ago

(took me a while to figure out that I had to pass the 'default' target to see something!)

Do you mean that just calling python -m pyblish_qml --demo isn't enough to see anything, that you also need --targets default? :O That shouldn't be the case, might be a bug.

buddly27 commented 4 years ago

Do you mean that just calling python -m pyblish_qml --demo isn't enough to see anything, that you also need --targets default? :O That shouldn't be the case, might be a bug.)

Yes that's what I mean. I guess it could be because targets default to ["default"] in pyblish_qml.host.show, but not in pyblish_qml.ipc.server.pyblish_qml. Though I feel default targets should probably be handled by pyblish-base instead for a better separation of concern between UI and logic, don't you think?

https://github.com/pyblish/pyblish-qml/blob/18826c8cad919b61e2c5332055242f85db814fd6/pyblish_qml/host.py#L100

https://github.com/pyblish/pyblish-qml/blob/18826c8cad919b61e2c5332055242f85db814fd6/pyblish_qml/ipc/server.py#L206

mottosso commented 4 years ago

Yes that's what I mean.

Can confirm this is happening, it happened in the latest release 1.11.1 and works as intended in 1.11.0. I've pushed a fix for it now.

https://github.com/pyblish/pyblish-qml/commit/835bf3bcc210fb181392502159438237597698ec

That's beside the point in this PR however. Let me know when you're ready for another review.

buddly27 commented 4 years ago

Can confirm this is happening, it happened in the latest release 1.11.1 and works as intended in 1.11.0. I've pushed a fix for it now.

Thanks!

Let me know when you're ready for another review.

Ready now :)

buddly27 commented 4 years ago

Just merged back latest change to this branch, let me know if there are any issues still!