-
Notifications
You must be signed in to change notification settings - Fork 119
SG-39209 New core patcher for PySide2 to look like PySide6 #1026
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
base: master
Are you sure you want to change the base?
Changes from all commits
df9693d
9071cf9
44f068b
6914e70
d8811ab
2f2cd2b
9e92634
8fa02af
c54ad07
d59805a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,8 @@ | |||||
| QtCore, | ||||||
| QtNetwork, | ||||||
| QtWebEngineWidgets, | ||||||
| QtWidgets, | ||||||
| QtWebEngineCore, | ||||||
| qt_version_tuple, | ||||||
| ) | ||||||
| from . import app_session_launcher | ||||||
|
|
@@ -114,7 +116,8 @@ def run(self): | |||||
| """ | ||||||
| self._site_info.reload(self._url_to_test, self._http_proxy) | ||||||
|
|
||||||
| class LoginDialog(QtGui.QDialog): | ||||||
|
|
||||||
| class LoginDialog(QtWidgets.QDialog): | ||||||
| """ | ||||||
| Dialog for getting user credentials. | ||||||
| """ | ||||||
|
|
@@ -144,13 +147,15 @@ def __init__( | |||||
| :param parent: The Qt parent for the dialog (defaults to None) | ||||||
| :param session_metadata: Metadata used in the context of SSO. This is an obscure blob of data. | ||||||
| """ | ||||||
| QtGui.QDialog.__init__(self, parent) | ||||||
| super().__init__(parent) | ||||||
|
|
||||||
| qt_modules = { | ||||||
| "QtCore": QtCore, | ||||||
| "QtGui": QtGui, | ||||||
| "QtNetwork": QtNetwork, | ||||||
| "QtWidgets": QtWidgets, | ||||||
| "QtWebEngineWidgets": QtWebEngineWidgets, | ||||||
| "QtWebEngineCore": QtWebEngineCore, | ||||||
| } | ||||||
| try: | ||||||
| self._sso_saml2 = SsoSaml2Toolkit( | ||||||
|
|
@@ -245,7 +250,7 @@ def __init__( | |||||
| self.ui.stackedWidget.setCurrentWidget(self.ui.login_page) | ||||||
|
|
||||||
| # Initialize Options menu | ||||||
| menu = QtGui.QMenu(self.ui.button_options) | ||||||
| menu = QtWidgets.QMenu(self.ui.button_options) | ||||||
| self.ui.button_options.setMenu(menu) | ||||||
| self.ui.button_options.setVisible(False) | ||||||
|
|
||||||
|
|
@@ -321,11 +326,11 @@ def __init__( | |||||
| ) | ||||||
|
|
||||||
| # Initialize exit confirm message box | ||||||
| self.confirm_box = QtGui.QMessageBox( | ||||||
| QtGui.QMessageBox.Question, | ||||||
| self.confirm_box = QtWidgets.QMessageBox( | ||||||
| QtWidgets.QMessageBox.Question, | ||||||
| "Flow Production Tracking Login", # title | ||||||
| "Would you like to cancel your request?", # text | ||||||
| buttons=QtGui.QMessageBox.Yes | QtGui.QMessageBox.No, | ||||||
| buttons=QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No, | ||||||
| # parent=self, | ||||||
| # Passing the parent parameter here, in the constructor, makes | ||||||
| # Nuke versions<=13 crash. | ||||||
|
|
@@ -353,7 +358,7 @@ def __del__(self): | |||||
| self._query_task.wait() | ||||||
|
|
||||||
| def _confirm_exit(self): | ||||||
| return self.confirm_box.exec_() == QtGui.QMessageBox.StandardButton.Yes | ||||||
| return self.confirm_box.exec_() == QtWidgets.QMessageBox.StandardButton.Yes | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| # PySide uses "exec_" instead of "exec" because "exec" is a reserved | ||||||
| # keyword in Python 2. | ||||||
|
|
||||||
|
|
@@ -491,15 +496,21 @@ def _toggle_web(self, method_selected=None): | |||||
| # - they need to use the legacy login / passphrase to use a PAT with | ||||||
| # Autodesk Identity authentication | ||||||
| if os.environ.get("SGTK_FORCE_STANDARD_LOGIN_DIALOG"): | ||||||
| logger.info("Using the standard login dialog with the Flow Production Tracking") | ||||||
| logger.info( | ||||||
| "Using the standard login dialog with the Flow Production Tracking" | ||||||
| ) | ||||||
| else: | ||||||
| if _is_running_in_desktop(): | ||||||
| can_use_web = can_use_web or self.site_info.autodesk_identity_enabled | ||||||
| can_use_web = ( | ||||||
| can_use_web or self.site_info.autodesk_identity_enabled | ||||||
| ) | ||||||
|
|
||||||
| # If we have full support for Web-based login, or if we enable it in our | ||||||
| # environment, use the Unified Login Flow for all authentication modes. | ||||||
| if get_shotgun_authenticator_support_web_login(): | ||||||
| can_use_web = can_use_web or self.site_info.unified_login_flow_enabled | ||||||
| can_use_web = ( | ||||||
| can_use_web or self.site_info.unified_login_flow_enabled | ||||||
| ) | ||||||
|
|
||||||
| if method_selected: | ||||||
| # Selecting requested mode (credentials, qt_web_login or app_session_launcher) | ||||||
|
|
@@ -511,9 +522,7 @@ def _toggle_web(self, method_selected=None): | |||||
| method_selected = session_cache.get_preferred_method(site) | ||||||
|
|
||||||
| # Make sure that the method_selected is currently supported | ||||||
| if ( | ||||||
| method_selected == auth_constants.METHOD_WEB_LOGIN and not can_use_web | ||||||
| ) or ( | ||||||
| if (method_selected == auth_constants.METHOD_WEB_LOGIN and not can_use_web) or ( | ||||||
| method_selected == auth_constants.METHOD_ASL and not can_use_asl | ||||||
| ): | ||||||
| method_selected = None | ||||||
|
|
@@ -525,9 +534,7 @@ def _toggle_web(self, method_selected=None): | |||||
| ) | ||||||
|
|
||||||
| # Make sure that the method_selected is currently supported | ||||||
| if ( | ||||||
| method_selected == auth_constants.METHOD_WEB_LOGIN and not can_use_web | ||||||
| ) or ( | ||||||
| if (method_selected == auth_constants.METHOD_WEB_LOGIN and not can_use_web) or ( | ||||||
| method_selected == auth_constants.METHOD_ASL and not can_use_asl | ||||||
| ): | ||||||
| method_selected = None | ||||||
|
|
@@ -667,7 +674,7 @@ def exec_(self): | |||||
| # to freeze, so only set the WindowStaysOnTopHint flag as this appears to not disable the | ||||||
| # other flags. | ||||||
| self.setWindowFlags(QtCore.Qt.WindowStaysOnTopHint) | ||||||
| return QtGui.QDialog.exec_(self) | ||||||
| return QtWidgets.QDialog.exec_(self) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to Copilot, on this same line, asking if the No, the exec_ method is not deprecated in PySide6, but the preferred method name is now exec. In PySide6 (and PyQt6), exec_() is still available for backward compatibility, but the trailing underscore was originally used because exec is a reserved keyword in Python 2. Since Python 3, exec is no longer a reserved word, so you can use exec() directly. So, for PySide6, you should use: return QtWidgets.QDialog.exec(self)The exec_() method will still work in PySide6 for now, but using exec() is the modern and recommended approach. If you want your code to be future-proof and style-compliant with the latest PySide6 standards, switch to exec(). Summary:
However, reading the documentation and not relying only on AI (I should've done this in the first place to avoid double lookup), I realized that we're encouraged to use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed explanation @carlos-villavicencio-adsk! However, switching from |
||||||
|
|
||||||
| def result(self): | ||||||
| """ | ||||||
|
|
@@ -688,21 +695,23 @@ def result(self): | |||||
| profile_location=profile_location, | ||||||
| ) | ||||||
| # If the offscreen session renewal failed, show the GUI as a failsafe | ||||||
| if res != QtGui.QDialog.Accepted: | ||||||
| if res != QtWidgets.QDialog.Accepted: | ||||||
| return | ||||||
|
|
||||||
| return self._sso_saml2.get_session_data() | ||||||
|
|
||||||
| res = self.exec_() | ||||||
| if res != QtGui.QDialog.Accepted: | ||||||
| if res != QtWidgets.QDialog.Accepted: | ||||||
| return | ||||||
|
|
||||||
| metrics_cache.log( | ||||||
| EventMetric.GROUP_TOOLKIT, | ||||||
| "Logged In", | ||||||
| properties={ | ||||||
| "authentication_method": self.site_info.user_authentication_method, | ||||||
| "authentication_experience": auth_constants.method_resolve.get(self.method_selected), | ||||||
| "authentication_experience": auth_constants.method_resolve.get( | ||||||
| self.method_selected | ||||||
| ), | ||||||
| "authentication_interface": "qt_dialog", | ||||||
| "authentication_renewal": self._is_session_renewal, | ||||||
| }, | ||||||
|
|
@@ -747,15 +756,15 @@ def _ok_pressed(self): | |||||
| Validate the values, accepting if login is successful and display an error message if not. | ||||||
| """ | ||||||
| # Wait for any ongoing Site Configuration check thread. | ||||||
| QtGui.QApplication.setOverrideCursor(QtCore.Qt.WaitCursor) | ||||||
| QtWidgets.QApplication.setOverrideCursor(QtCore.Qt.WaitCursor) | ||||||
| try: | ||||||
| if not self._query_task.wait(THREAD_WAIT_TIMEOUT_MS): | ||||||
| logger.warning( | ||||||
| "Timed out awaiting configuration information on the site: %s" | ||||||
| % self._get_current_site() | ||||||
| ) | ||||||
| finally: | ||||||
| QtGui.QApplication.restoreOverrideCursor() | ||||||
| QtWidgets.QApplication.restoreOverrideCursor() | ||||||
|
|
||||||
| # pull values from the gui | ||||||
| site = self._get_current_site() | ||||||
|
|
@@ -771,7 +780,10 @@ def _ok_pressed(self): | |||||
|
|
||||||
| # Cleanup the URL and update the GUI. | ||||||
| if self.method_selected != auth_constants.METHOD_BASIC: | ||||||
| if site.startswith("http://") and "SGTK_AUTH_ALLOW_NO_HTTPS" not in os.environ: | ||||||
| if ( | ||||||
| site.startswith("http://") | ||||||
| and "SGTK_AUTH_ALLOW_NO_HTTPS" not in os.environ | ||||||
| ): | ||||||
| site = "https" + site[4:] | ||||||
| self.ui.site.setEditText(site) | ||||||
|
|
||||||
|
|
@@ -827,7 +839,7 @@ def _authenticate(self, error_label, site, login, password, auth_code=None): | |||||
| product=PRODUCT_IDENTIFIER, | ||||||
| profile_location=profile_location, | ||||||
| ) | ||||||
| if res == QtGui.QDialog.Accepted: | ||||||
| if res == QtWidgets.QDialog.Accepted: | ||||||
| self._new_session_token = self._sso_saml2.session_id | ||||||
| self._session_metadata = self._sso_saml2.cookies | ||||||
| else: | ||||||
|
|
@@ -837,8 +849,8 @@ def _authenticate(self, error_label, site, login, password, auth_code=None): | |||||
| return | ||||||
| else: | ||||||
| # set the wait cursor | ||||||
| QtGui.QApplication.setOverrideCursor(QtCore.Qt.WaitCursor) | ||||||
| QtGui.QApplication.processEvents() | ||||||
| QtWidgets.QApplication.setOverrideCursor(QtCore.Qt.WaitCursor) | ||||||
| QtWidgets.QApplication.processEvents() | ||||||
|
|
||||||
| # try and authenticate | ||||||
| self._new_session_token = session_cache.generate_session_token( | ||||||
|
|
@@ -851,9 +863,9 @@ def _authenticate(self, error_label, site, login, password, auth_code=None): | |||||
| success = True | ||||||
| finally: | ||||||
| # restore the cursor | ||||||
| QtGui.QApplication.restoreOverrideCursor() | ||||||
| QtWidgets.QApplication.restoreOverrideCursor() | ||||||
| # dialog is done | ||||||
| QtGui.QApplication.processEvents() | ||||||
| QtWidgets.QApplication.processEvents() | ||||||
|
|
||||||
| # Do not accept while the cursor is overriden, if freezes the dialog. | ||||||
| if success: | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-alpha order