Skip to content
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

Fix #360 - quit Pyblish QML when Blender's operator is cancelled #361

Merged
merged 2 commits into from
Mar 9, 2020

Conversation

jasperges
Copy link
Contributor

This resolves #360

@mottosso
Copy link
Member

mottosso commented Mar 4, 2020

Nice one, but do still keep the reference to the server so it's clear what "quit()" refers to. It's a little too similar to exit() which would kill the Python interpreter.

@jasperges
Copy link
Contributor Author

The reason I just call quit() is twofold:

  1. The example code you shared in Pyblish QML is unresponsive when opening another file in Blender #360, didn't work, because it calls quit() on the server and not on the proxy.
  2. While looking what went wrong with 1, I discovered the quit() function in host.py. That function does exactly what I need via the proxy_call wrapper.

So I'm not sure how you would like me to change this. By using the proxy_call wrapper there is an extra safety check to see if the server is actually still alive. Even though the chances are very slim (or even non-existent) that the server is gone when the operator is not cancelled yet, I think it's a good idea to have this check. To keep a reference to the proxy I would have to duplicate this code, which seems like a bad idea.

@mottosso
Copy link
Member

mottosso commented Mar 5, 2020

The example code you shared in #360, didn't work, because it calls quit() on the server and not on the proxy.

Eeek. The terminology here isn't great; the "proxy" wraps the "server" with commands, like quit. Server in this case is Pyblish QML, where Blender is the client.

Looks like I left out a line from my example.

proxy = ipc.server.Proxy(server)

With that, you should be able to call on proxy.quit().

While looking what went wrong with 1, I discovered the quit() function in host.py

Double-eeek! I had also not seen this function. It's a context manager for a wrapper of a wrapper for the server. The proxy should be handling where the server has been killer or is missing.

Let's stick with calling proxy.quit() directly, so that we can migrate away from those free-floating functions that have no business being in host.py (but should be in ipc/server.py).

@jasperges
Copy link
Contributor Author

Okay, thanks for clarifying. I completely agree that proxy.quit() is better. It makes it clear what has to quit.

I have another question: api.current_server() calls host.current_server(). Is it okay to directly call current_server() or do you prefer to do it via api, so things can be moved around without breaking this code?
And is it okay if I do from . import api?

So many questions for such a simple thing... 😅

@mottosso
Copy link
Member

mottosso commented Mar 5, 2020

The API is only for external use. Internal modules shouldn't call it, unless it's an even higher-level of abstraction than the API (like e.g. pyblish.util).

@jasperges
Copy link
Contributor Author

Sorry, now I'm confused. Should I use api.current_server() or just current_server()?

@mottosso
Copy link
Member

mottosso commented Mar 5, 2020

I would do what other functions in the module do, unless there's a reason not to.

server = _state["currentServer"]
proxy = ipc.server.Proxy(server)

@jasperges
Copy link
Contributor Author

To be on the safe side, I use:

server = _state.get("currentServer")
if server:
    proxy = ipc.server.Proxy(server)
    proxy.quit()

@mottosso
Copy link
Member

mottosso commented Mar 9, 2020

Looks good, unsure of why the Docker image fails for the tests.. hopefully it's temporary and doesn't involve this change. Thanks @jasperges

@mottosso mottosso merged commit f59b1ab into pyblish:master Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pyblish QML is unresponsive when opening another file in Blender
2 participants