NXENG-627: PySide Migration - #6450
Open
S-Ghosh-Dev2 wants to merge 21 commits into
Open
Conversation
…ration-share-debug-info-dialog' into poc/pyside
S-Ghosh-Dev2
requested review from
gitofanindya
and
a lite review from Copilot
August 24, 2026 11:06
Contributor
Reviewer's GuideThis PR migrates the application and Direct Transfer flow from PyQt6 to PySide6 via a centralized compatibility shim, updates Qt-specific signal/model and frozen-build lifecycle behavior, introduces a temporary Settings QML adapter POC, and aligns dependencies, deployment scripts, tests, and documentation with the new binding. Sequence diagram for Settings section routingsequenceDiagram
participant User
participant Tray as Systray
participant Application
participant Settings as Main_QML_Settings_window
User->>Tray: activate
Tray->>Application: show_settings(section)
Application->>Application: setSection.emit(section)
Application->>Application: _center_on_screen(settings_window)
Application->>Settings: show
alt unknown section
Application->>Application: show_settings(unknown_section)
Application->>Application: setSection.emit(Features)
end
Sequence diagram for account error Settings routingsequenceDiagram
participant QML as Settings_QML
participant API as QMLDriveApi
participant Application
participant Settings as Main_QML_Settings_window
QML->>API: bind_server(server_url, local_folder, token, check_fs)
API->>Application: show_settings(Accounts)
Application->>Settings: setSection.emit(Accounts)
Application->>Application: _center_on_screen(settings_window)
Application->>Settings: show
Flow diagram for PySide6 model and type compatibility updatesflowchart TD
QML[Direct Transfer QML]
Models[DirectTransferModel and ActiveSessionModel]
Roles[dataChanged role ID list]
FolderTree[Folder tree loaders]
PythonData[Direct Python objects]
Schedule[ScheduleDialog]
PythonDate["QDateTime.toPython()"]
QML --> Models
Models --> Roles
FolderTree --> PythonData
Schedule --> PythonDate
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. A faulty PySide6 migration or incomplete packaging can prevent the application from starting or leave core UI, QML, signal delivery, or Direct Transfer flows unusable in a released build, causing an outage before the release is rolled back. Reverting restores the prior PyQt6 implementation for future launches, but it cannot undo downtime or failed user operations that occurred while the broken build was deployed.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6450 +/- ##
==========================================
- Coverage 98.93% 96.85% -2.09%
==========================================
Files 135 136 +1
Lines 23184 23572 +388
==========================================
- Hits 22937 22830 -107
- Misses 247 742 +495
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the runtime migration from PyQt6 to PySide6 by switching the project-wide Qt shim (nxdrive.drive.qt.imports) to PySide6 and updating application code, tests, and deployment checks accordingly, with additional compatibility work for signals/slots, QML, and frozen builds.
Changes:
- Replace PyQt6 runtime usage with PySide6 (including shim exports, slot registration, and PySide6-specific API adjustments like
QDateTime.toPython()anddataChangedrole lists). - Update unit/functional/integration tests and patch targets to use PySide6 or the shared shim, and improve test isolation around Qt threading/event-loop behavior.
- Update deploy scripts and pinned requirements to remove PyQt6 and require PySide6 + Shiboken, plus add migration documentation.
Reviewed changes
Copilot reviewed 42 out of 55 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/windows/deploy_ci_agent.ps1 | Remove PyQt6 Bluetooth workaround; verify PySide6 import post-install. |
| tools/posix/deploy_ci_agent.sh | Verify PySide6 import post-install instead of PyQt6. |
| tools/deps/requirements.txt | Swap PyQt6 pins/hashes for PySide6 + addons/essentials + shiboken6. |
| tests/nuxeo/unit/gui/test_api.py | Expect Application.show_settings("Accounts") instead of _show_window(settings_window). |
| tests/nuxeo/functional/gui/test_application.py | Move Qt imports to shim; update patch targets from PyQt6 to PySide6. |
| tests/conftest.py | Switch shared test Qt app fixture from QCoreApplication to QApplication. |
| tests/common/unit/test_tracing.py | Clear internal tracing event cache before/after fixture for isolation. |
| tests/common/unit/test_local_watcher.py | Force direct Qt signal delivery in tests via DirectConnection. |
| tests/common/unit/test_direct_edit_extra.py | Force direct Qt signal delivery in tests via DirectConnection. |
| tests/common/unit/test_direct_download_extra.py | Force direct Qt signal delivery in tests via DirectConnection. |
| tests/common/unit/test_commandline.py | Update patch target to PySide6.QtNetwork.QLocalSocket. |
| tests/common/unit/test_autolock.py | Force direct Qt signal delivery in tests via DirectConnection. |
| tests/common/unit/gui/test_multi_folder_dialog.py | Replace patching of C++ virtual methods with Python subclass overrides. |
| tests/common/unit/gui/test_folders_dialog.py | Update comment wording for Qt init check workaround. |
| tests/common/unit/gui/test_folders_dialog_extra.py | Adjust mocked QDateTime conversion to toPython(). |
| tests/common/unit/gui/test_application.py | Add method-host indirection to avoid creating extra QApplication instances in tests. |
| tests/common/unit/gui/test_application_gap_coverage.py | Add slot/metaobject regression tests; add method-host approach for Application gap coverage. |
| tests/common/unit/gui/test_api_extra.py | Expect Application.show_settings("Accounts") instead of _show_window(settings_window). |
| tests/common/integration/macos/test_show_hide_refresh_button.py | Replace PyQt6 import with shim import. |
| tests/common/integration/macos/test_keyPressEvent.py | Replace PyQt6 import with shim import. |
| tests/common/functional/mocked_classes.py | Remove PyQt6 imports; use shim Qt types and signal aliases in mocks. |
| tests/alfresco/unit/test_oauth2.py | Remove PyQt6 module injection; patch shim symbols directly; adjust base-class patching. |
| tests/alfresco/functional/gui/test_application.py | Import-skip PySide6 instead of PyQt6. |
| nxdrive/drive/qt/imports.py | Switch shim exports to PySide6; keep pyqt* aliases; add curated PySide namespace; adjust exports. |
| nxdrive/drive/qt/constants.py | Source constants from shim and add compatibility PySide enum namespace. |
| nxdrive/drive/qt/init.py | Update module docs for binding switching and reference shim env var. |
| nxdrive/drive/manager.py | Register additional methods as Qt slots. |
| nxdrive/drive/gui/view.py | Emit list-of-role-ids in dataChanged (PySide6-compatible). |
| nxdrive/drive/gui/systray.py | Register native callback methods as Qt slots with explicit signatures. |
| nxdrive/drive/gui/multi_folder_dialog.py | Update dark-mode comment wording (binding-agnostic). |
| nxdrive/drive/gui/folders_loader.py | Remove QVariant wrapping; store Python objects directly via setData. |
| nxdrive/drive/gui/folders_dialog.py | Switch QDateTime conversion to toPython() and cast for typing. |
| nxdrive/drive/gui/custom_window.py | Register visibility callback as a Qt slot (PySide6 metaobject compatibility). |
| nxdrive/drive/gui/application.py | Add Settings section routing fallback; guard systray window close; avoid auto-show Settings at startup; change update notification frozen guard; improve logging; make metrics dialog icon self-contained. |
| nxdrive/drive/gui/api.py | Use Application.show_settings("Accounts") for error routing. |
| nxdrive/drive/engine/engine.py | Align slot signature usage; add Qt slot decorators for timer/session lifecycle methods. |
| nxdrive/drive/direct_edit.py | Convert overloaded signal declaration to PySide6 tuple overload syntax. |
| nxdrive/drive/data/icons/README.md | Remove icons README (asset cleanup). |
| nxdrive/drive/data/icons/paused.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/paused_light.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/idle.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/idle_light.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/error.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/error_light.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/emblem.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/conflict.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/conflict_light.svg | Remove obsolete icon asset. |
| nxdrive/drive/data/icons/active.svg | Remove obsolete icon asset. |
| nxdrive/alfresco/auth/oauth2.py | Use shim Qt imports for loopback flow QObject/signal types. |
| nxdrive/main.py | Clarify Qt Quick Controls style env var comment for both bindings. |
| docs/pyside6_migration.md | Add migration ledger documenting decisions, validation, and remaining work. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 57 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
docs/pyside6_migration.md:83
- This validation bullet is inaccurate: the repo still contains PyQt6 imports (e.g.
nxdrive/drive/gui/pyside_settings_host.pycurrently describes a mixed PyQt6/PySide6 setup and includes PyQt6-era references). Please adjust the statement to reflect what was actually scanned/validated or explicitly exclude that legacy, unused module.
- Source scan: no executable PyQt6 imports remain under `nxdrive`, `tests`, or `tools`.
tests/conftest.py:252
- The
appfixture now creates aQApplicationbut does not setQT_QPA_PLATFORM=offscreenwhen running headless on Linux. In CI environments withoutDISPLAY, this can cause Qt platform plugin initialization failures when the fixture is first used.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 58 changed files in this pull request and generated no new comments.
Suppressed comments (4)
Previously missed (4) — in code that hasn't changed since the last review.
nxdrive/drive/gui/api.py:1052
- The comment says “Arise the settings window…”, but the correct verb here is “Raise”. This reads like a typo and can confuse future readers.
# Arise the settings window to let the user know the error.
nxdrive/drive/qt/init.py:5
- This module docstring references an
NXDRIVE_QT_BINDINGenv var, but there are no other references/implementation of that env var anywhere in the repo. This makes the documentation misleading.
tests/alfresco/unit/test_oauth2.py:772 - Using
type(auth).__mro__[1]assumesAlfrescoOAuthenticationwill always directly inherit the class you want to patch. That makes the test brittle (any future mixin/reordering breaks it). Import and patchOAuthenticationBasedirectly instead.
nxdrive/main.py:30 - This comment mentions selecting the active binding via
NXDRIVE_QT_BINDING, but that env var is not implemented/referenced anywhere else in the repo. The comment should be updated to avoid implying a runtime switch that doesn’t exist.
# The same env var is honored by both PyQt6 and PySide6 Qt Quick engines, so we
# set it regardless of the active binding selected via ``NXDRIVE_QT_BINDING``.
sourcery-ai
Bot
dismissed
their stale review
August 25, 2026 08:51
Sourcery withdrew this approval because the latest commits introduced blocking findings.
Comment on lines
175
to
+177
| Application._workflow_cls # Set at init, used by workflow dispatch | ||
| PySideSettingsHost # Retained for the deferred Settings host cleanup | ||
| PySideSettingsHost._setmessage_bridge # Keeps the QML relay callback alive |
|
|
||
| @pyqtSlot(object) | ||
| @Slot(object) | ||
| def _check_sync_start(self, *, row_id: str = None) -> None: |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 94 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
tests/common/functional/mocked_classes.py:523
Mock_Qt.setMessageis assigned to theSignalInstancetype, not an instance/callable. InApplication.init_gui()the code connects a real Qt signal to the QML root’ssetMessagetarget; using the class here can lead to confusing runtime behavior (e.g., the signal invoking theSignalInstanceconstructor) instead of safely absorbing the call in tests.
This issue also appears on line 526 of the same file.
tests/common/functional/mocked_classes.py:527
Mock_Qt.updateAvailable/updateProgressare also set to theSignalInstancetype. These attributes are used as signal targets during GUI setup, so they should be safe callables (or real mock signal objects) to avoid unexpected type-construction when the updater emits signals in tests.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 94 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/common/functional/mocked_classes.py:527
Mock_Qt.setMessage,updateAvailable, andupdateProgressare assigned to theSignalInstancetype rather than a callable/signal-like instance. In the migratedApplication.init_gui()these attributes are used as Qt signal targets (e.g.api.setMessage.connect(root.setMessage)andupdater.updateAvailable.connect(root.updateAvailable)), so passing a class object here can raise at connect/emit time or silently do the wrong thing (instantiating the type). Provide a small callable stub that has bothemit(...)and__call__(...)so it can act like a QML signal/slot during tests.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 94 changed files in this pull request and generated no new comments.
Suppressed comments (1)
nxdrive/alfresco/engine/engine.py:164
AlfrescoEngine._check_sync_startdeclaresrow_idas a keyword-only parameter (*), but it’s connected in the baseEngine.__init__viaself.newQueueItem.connect(self._check_sync_start)/queue_manager.newItem.connect(self._check_sync_start), which will invoke the slot with a positional argument. With the current signature, that call will raiseTypeError: got an unexpected positional argumentat runtime when the signal emits.
@Slot(object)
def _check_sync_start(self, *, row_id: str = None) -> None:
if not self._sync_started:
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 82 out of 95 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tools/skiplist.py:177
tools/skiplist.pyreferencesPySideSettingsHost._setmessage_bridge, but_setmessage_bridgeis an instance attribute (initialized inPySideSettingsHost.__init__), not a class attribute. As written, importing/executingtools/skiplist.pywill raiseAttributeErrorand break whatever tooling consumes this skiplist.
Use a safe getattr(...) (or drop the attribute reference entirely) so the module can be evaluated without errors.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 82 out of 95 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/common/functional/mocked_classes.py:527
Mock_QtassignssetMessage,updateAvailable, andupdateProgressto theSignalInstancetype instead of a signal-like object. Code under test expects QML/root signal objects with.emit()(and potentially.connect()), so this can raiseAttributeError/TypeErrorwhen wiring signals (e.g.,api.setMessage.connect(root.setMessage)or emittingroot.setMessage.emit(...)). Use the existingMock_Emitter()(or aMagicMockwithemit) to model these signals.
S-Ghosh-Dev2
force-pushed
the
NXENG-627-pyside-migration
branch
from
August 28, 2026 10:19
180f167 to
78d0451
Compare
Comment on lines
+523
to
+527
| self.setMessage = SignalInstance | ||
| self.setSection = Mock_Emitter() | ||
| self.setStatus = self | ||
| self.updateAvailable: QtCore.PYQT_SLOT = QtCore.pyqtBoundSignal | ||
| self.updateProgress: QtCore.PYQT_SLOT = QtCore.pyqtBoundSignal | ||
| self.updateAvailable = SignalInstance | ||
| self.updateProgress = SignalInstance |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Complete the application-wide migration from PyQt6 to PySide6 while consolidating QML window management and updating build, deployment, documentation, and test infrastructure.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Deployment:
Documentation:
Tests: