From 0a4be2b195b2ff87faa856cabeae573aeb9ae5b0 Mon Sep 17 00:00:00 2001 From: Chidi Williams Date: Wed, 26 Apr 2023 22:10:42 +0000 Subject: [PATCH] Try to load API key only when needed (#420) --- .github/workflows/ci.yml | 2 +- buzz/gui.py | 28 ++++++++----------- buzz/store/keyring_store.py | 11 ++++---- buzz/widgets/general_preferences_widget.py | 10 ++++--- buzz/widgets/preferences_dialog.py | 6 ++-- .../general_preferences_widget_test.py | 12 ++++++-- tests/widgets/preferences_dialog_test.py | 2 +- 7 files changed, 38 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 07c8955d..bb78a461 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -180,7 +180,7 @@ jobs: benchmark: runs-on: ${{ matrix.os }} - if: github.ref == 'refs/heads/master' + if: github.ref == 'refs/heads/main' strategy: fail-fast: false matrix: diff --git a/buzz/gui.py b/buzz/gui.py index 13722106..e34f50cd 100644 --- a/buzz/gui.py +++ b/buzz/gui.py @@ -211,12 +211,14 @@ class FileTranscriberWidget(QWidget): openai_access_token_changed = pyqtSignal(str) settings = Settings() - def __init__(self, file_paths: List[str], openai_access_token: Optional[str] = None, - parent: Optional[QWidget] = None, flags: Qt.WindowType = Qt.WindowType.Widget) -> None: + def __init__(self, file_paths: List[str], parent: Optional[QWidget] = None, + flags: Qt.WindowType = Qt.WindowType.Widget) -> None: super().__init__(parent, flags) self.setWindowTitle(file_paths_as_title(file_paths)) + openai_access_token = KeyringStore().get_password(KeyringStore.Key.OPENAI_API_KEY) + self.file_paths = file_paths default_language = self.settings.value(key=Settings.Key.FILE_TRANSCRIBER_LANGUAGE, default_value='') self.transcription_options = TranscriptionOptions( @@ -909,8 +911,6 @@ class MainWindow(QMainWindow): self.tasks_cache = tasks_cache - self.openai_access_token = KeyringStore.get_password(KeyringStore.Key.OPENAI_API_KEY) - self.settings = Settings() self.shortcut_settings = ShortcutSettings(settings=self.settings) @@ -927,7 +927,7 @@ class MainWindow(QMainWindow): self.addToolBar(self.toolbar) self.setUnifiedTitleAndToolBarOnMac(True) - self.menu_bar = MenuBar(shortcuts=self.shortcuts, openai_api_key=self.openai_access_token, parent=self) + self.menu_bar = MenuBar(shortcuts=self.shortcuts, parent=self) self.menu_bar.import_action_triggered.connect( self.on_new_transcription_action_triggered) self.menu_bar.shortcuts_changed.connect(self.on_shortcuts_changed) @@ -1022,17 +1022,16 @@ class MainWindow(QMainWindow): def open_file_transcriber_widget(self, file_paths: List[str]): file_transcriber_window = FileTranscriberWidget(file_paths=file_paths, - openai_access_token=self.openai_access_token, parent=self, + parent=self, flags=Qt.WindowType.Window) file_transcriber_window.triggered.connect( self.on_file_transcriber_triggered) file_transcriber_window.openai_access_token_changed.connect(self.on_openai_access_token_changed) file_transcriber_window.show() - def on_openai_access_token_changed(self, access_token: str): - self.openai_access_token = access_token - self.menu_bar.set_openai_api_key(self.openai_access_token) - KeyringStore.set_password(KeyringStore.Key.OPENAI_API_KEY, access_token) + @staticmethod + def on_openai_access_token_changed(access_token: str): + KeyringStore().set_password(KeyringStore.Key.OPENAI_API_KEY, access_token) def open_transcript_viewer(self): selected_rows = self.table_widget.selectionModel().selectedRows() @@ -1361,11 +1360,10 @@ class MenuBar(QMenuBar): shortcuts_changed = pyqtSignal(dict) openai_api_key_changed = pyqtSignal(str) - def __init__(self, shortcuts: Dict[str, str], openai_api_key: str, parent: QWidget): + def __init__(self, shortcuts: Dict[str, str], parent: QWidget): super().__init__(parent) self.shortcuts = shortcuts - self.openai_api_key = openai_api_key self.import_action = QAction(_("Import Media File..."), self) self.import_action.triggered.connect( @@ -1394,8 +1392,7 @@ class MenuBar(QMenuBar): about_dialog.open() def on_preferences_action_triggered(self): - preferences_dialog = PreferencesDialog(shortcuts=self.shortcuts, openai_api_key=self.openai_api_key, - parent=self) + preferences_dialog = PreferencesDialog(shortcuts=self.shortcuts, parent=self) preferences_dialog.shortcuts_changed.connect(self.shortcuts_changed) preferences_dialog.openai_api_key_changed.connect(self.openai_api_key_changed) preferences_dialog.open() @@ -1406,9 +1403,6 @@ class MenuBar(QMenuBar): self.import_action.setShortcut(QKeySequence.fromString(shortcuts[Shortcut.OPEN_IMPORT_WINDOW.name])) self.preferences_action.setShortcut(QKeySequence.fromString(shortcuts[Shortcut.OPEN_PREFERENCES_WINDOW.name])) - def set_openai_api_key(self, key: str): - self.openai_api_key = key - class Application(QApplication): window: MainWindow diff --git a/buzz/store/keyring_store.py b/buzz/store/keyring_store.py index 7eeec54b..1b4adb19 100644 --- a/buzz/store/keyring_store.py +++ b/buzz/store/keyring_store.py @@ -11,16 +11,17 @@ class KeyringStore: class Key(enum.Enum): OPENAI_API_KEY = 'OpenAI API key' - @staticmethod - def get_password(username: Key) -> str: + def get_password(self, username: Key) -> str: try: - return keyring.get_password(APP_NAME, username=username.value) + password = keyring.get_password(APP_NAME, username=username.value) + if password is None: + return '' + return password except (KeyringLocked, KeyringError) as exc: logging.error('Unable to read from keyring: %s', exc) return '' - @staticmethod - def set_password(username: Key, password: str) -> None: + def set_password(self, username: Key, password: str) -> None: try: keyring.set_password(APP_NAME, username.value, password) except (KeyringLocked, PasswordSetError) as exc: diff --git a/buzz/widgets/general_preferences_widget.py b/buzz/widgets/general_preferences_widget.py index 952db9e3..121dd5b8 100644 --- a/buzz/widgets/general_preferences_widget.py +++ b/buzz/widgets/general_preferences_widget.py @@ -3,22 +3,24 @@ from typing import Optional import openai from PyQt6.QtCore import QRunnable, QObject, pyqtSignal, QThreadPool -from PyQt6.QtWidgets import QWidget, QFormLayout, QLineEdit, QPushButton, QMessageBox +from PyQt6.QtWidgets import QWidget, QFormLayout, QPushButton, QMessageBox from openai.error import AuthenticationError +from buzz.store.keyring_store import KeyringStore from buzz.widgets.openai_api_key_line_edit import OpenAIAPIKeyLineEdit class GeneralPreferencesWidget(QWidget): openai_api_key_changed = pyqtSignal(str) - def __init__(self, openai_api_key: str, parent: Optional[QWidget] = None): + def __init__(self, keyring_store=KeyringStore(), parent: Optional[QWidget] = None): super().__init__(parent) - self.openai_api_key = openai_api_key + + self.openai_api_key = keyring_store.get_password(KeyringStore.Key.OPENAI_API_KEY) layout = QFormLayout(self) - self.openai_api_key_line_edit = OpenAIAPIKeyLineEdit(openai_api_key, self) + self.openai_api_key_line_edit = OpenAIAPIKeyLineEdit(self.openai_api_key, self) self.openai_api_key_line_edit.key_changed.connect(self.on_openai_api_key_changed) self.test_openai_api_key_button = QPushButton('Test') diff --git a/buzz/widgets/preferences_dialog.py b/buzz/widgets/preferences_dialog.py index 8a2bb55f..61694df0 100644 --- a/buzz/widgets/preferences_dialog.py +++ b/buzz/widgets/preferences_dialog.py @@ -4,6 +4,7 @@ from PyQt6.QtCore import pyqtSignal from PyQt6.QtWidgets import QDialog, QWidget, QVBoxLayout, QTabWidget, QDialogButtonBox from buzz.locale import _ +from buzz.store.keyring_store import KeyringStore from buzz.widgets.general_preferences_widget import GeneralPreferencesWidget from buzz.widgets.shortcuts_editor_preferences_widget import ShortcutsEditorPreferencesWidget @@ -12,7 +13,7 @@ class PreferencesDialog(QDialog): shortcuts_changed = pyqtSignal(dict) openai_api_key_changed = pyqtSignal(str) - def __init__(self, shortcuts: Dict[str, str], openai_api_key: str, parent: Optional[QWidget] = None) -> None: + def __init__(self, shortcuts: Dict[str, str], parent: Optional[QWidget] = None) -> None: super().__init__(parent) self.setWindowTitle('Preferences') @@ -20,8 +21,7 @@ class PreferencesDialog(QDialog): layout = QVBoxLayout(self) tab_widget = QTabWidget(self) - general_tab_widget = GeneralPreferencesWidget(openai_api_key=openai_api_key, - parent=self) + general_tab_widget = GeneralPreferencesWidget(parent=self) general_tab_widget.openai_api_key_changed.connect(self.openai_api_key_changed) tab_widget.addTab(general_tab_widget, _('General')) diff --git a/tests/widgets/general_preferences_widget_test.py b/tests/widgets/general_preferences_widget_test.py index a6b891bf..b826bef0 100644 --- a/tests/widgets/general_preferences_widget_test.py +++ b/tests/widgets/general_preferences_widget_test.py @@ -1,13 +1,15 @@ from unittest.mock import Mock +import pytest from PyQt6.QtWidgets import QPushButton, QMessageBox, QLineEdit +from buzz.store.keyring_store import KeyringStore from buzz.widgets.general_preferences_widget import GeneralPreferencesWidget class TestGeneralPreferencesWidget: def test_should_disable_test_button_if_no_api_key(self, qtbot): - widget = GeneralPreferencesWidget(openai_api_key='') + widget = GeneralPreferencesWidget(keyring_store=self.get_keyring_store('')) qtbot.add_widget(widget) test_button = widget.findChild(QPushButton) @@ -23,7 +25,7 @@ class TestGeneralPreferencesWidget: assert test_button.isEnabled() def test_should_test_openai_api_key(self, qtbot): - widget = GeneralPreferencesWidget(openai_api_key='wrong-api-key') + widget = GeneralPreferencesWidget(keyring_store=self.get_keyring_store('wrong-api-key')) qtbot.add_widget(widget) test_button = widget.findChild(QPushButton) @@ -41,3 +43,9 @@ class TestGeneralPreferencesWidget: 2] == 'Incorrect API key provided: wrong-ap*-key. You can find your API key at https://platform.openai.com/account/api-keys.' qtbot.waitUntil(mock_called) + + @staticmethod + def get_keyring_store(password: str): + keyring_store = Mock(KeyringStore) + keyring_store.get_password.return_value = password + return keyring_store diff --git a/tests/widgets/preferences_dialog_test.py b/tests/widgets/preferences_dialog_test.py index 90d01074..6e549013 100644 --- a/tests/widgets/preferences_dialog_test.py +++ b/tests/widgets/preferences_dialog_test.py @@ -6,7 +6,7 @@ from buzz.widgets.preferences_dialog import PreferencesDialog class TestPreferencesDialog: def test_create(self, qtbot: QtBot): - dialog = PreferencesDialog(shortcuts={}, openai_api_key='') + dialog = PreferencesDialog(shortcuts={}) qtbot.add_widget(dialog) assert dialog.windowTitle() == 'Preferences'