SG-36162 Refactor __launch_app_proxy_for_project in a QThread #187
Conversation
python/tk_desktop/desktop_window.py
Outdated
| @@ -157,7 +156,7 @@ class DesktopWindow(SystrayWindow): | |||
| _CHROME_SUPPORT_URL = "https://developer.shotgridsoftware.com/95518180" | |||
| _FIREFOX_SUPPORT_URL = "https://developer.shotgridsoftware.com/d4936105" | |||
|
|
|||
| def __init__(self, console, parent=None): | |||
| def __init__(self, console, task_manager, parent=None): | |||
There was a problem hiding this comment.
very small detail, it would be better to call this argument bg_task_manager rather than task_manager (easier to identify + this is usually how it is called)
python/tk_desktop/desktop_window.py
Outdated
| # trigger an update to the model to track this project access | ||
| self.__set_project_just_accessed(project) | ||
| QtGui.QApplication.instance().processEvents() | ||
| def __launch_app_proxy_for_project_follow_up(self): |
There was a problem hiding this comment.
Unlike @julien-lang , I don't think this method name is the best.. looking at the code, it's all about pipeline configuration and I find the "new name" kind of confusing. I'd prefer something like __load_pipeline_configuration() (+ add comments to the method/arguments to explain what this is about, especially is the method name is not descriptive :))
python/tk_desktop/desktop_window.py
Outdated
| if task_id != self.__set_project_just_accessed_task: | ||
| return | ||
|
|
||
| log.error(f"Launching app proxy for project failed. Message: {msg}") |
There was a problem hiding this comment.
you also need to clear self.__set_project_just_accessed_task if the task has failed
maybe you can also change the error message to have a better reflection of what has failed by replacing Launching app proxy for project failed by Error happened when loading pipeline configuration: {msg}
(tbh, as a user/artist, I don't know what is the app proxy :D )
also wondering if we could use self.project_overlay.show_error_message(message) to display the error message to the user as well?
There was a problem hiding this comment.
I've refactored the error handling on the callback and cleared the task. Good catch!
Move the logic for phase 1 (update last accessed timestamp in SG) when launching of the app proxy for a project in a
QThread