From c39468a6d067943d37d5127ecc0497f201c9ac38 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 1 Aug 2024 17:53:10 +0100 Subject: [PATCH 1/7] Update variables in GUI after saving context file --- damnit/ctxsupport/damnit_ctx.py | 2 ++ damnit/gui/main_window.py | 5 +++++ damnit/gui/table.py | 6 +++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/damnit/ctxsupport/damnit_ctx.py b/damnit/ctxsupport/damnit_ctx.py index 288f63de..ae1befee 100644 --- a/damnit/ctxsupport/damnit_ctx.py +++ b/damnit/ctxsupport/damnit_ctx.py @@ -50,6 +50,8 @@ def __init__( def __call__(self, func): self.func = func self.name = func.__name__ + if self.title is None: + self.title = self.name return self def check(self): diff --git a/damnit/gui/main_window.py b/damnit/gui/main_window.py index 7fad4bd7..b55e9115 100644 --- a/damnit/gui/main_window.py +++ b/damnit/gui/main_window.py @@ -50,6 +50,7 @@ class MainWindow(QtWidgets.QMainWindow): context_dir_changed = QtCore.pyqtSignal(str) save_context_finished = QtCore.pyqtSignal(bool) # True if saved + context_saved = QtCore.pyqtSignal() db = None db_id = None @@ -88,6 +89,8 @@ def __init__(self, context_dir: Path = None, connect_to_kafka: bool = True): self._tab_widget.setEnabled(False) self.setCentralWidget(self._tab_widget) + self.context_saved.connect(self.launch_update_computed_vars) + self.table = None self.zulip_messenger = None @@ -262,6 +265,7 @@ def autoconfigure(self, path: Path): self.launch_update_computed_vars() def launch_update_computed_vars(self): + # Triggered when we open a proposal & when saving the context file log.debug("Launching subprocess to read variables from context file") proc = QtCore.QProcess(parent=self) # Show stdout & stderr with the parent process @@ -812,6 +816,7 @@ def test_context_result(self, test_result, output, checked_code): self.mark_context_saved() self._context_code_to_save = None self.save_context_finished.emit(saving) + self.context_saved.emit() if test_result == ContextTestResult.ERROR: self.set_error_widget_text(output) diff --git a/damnit/gui/table.py b/damnit/gui/table.py index 88226505..3a062aec 100644 --- a/damnit/gui/table.py +++ b/damnit/gui/table.py @@ -115,7 +115,11 @@ def set_column_visibility(self, name, visible, for_restore=False): deselected. The `for_restore` argument lets you specify which behaviour you want. """ - column_index = self.damnit_model.find_column(name, by_title=True) + try: + column_index = self.damnit_model.find_column(name, by_title=True) + except KeyError: + log.error("Could not find column %r to set visibility", name) + return self.setColumnHidden(column_index, not visible) From a9e2fcc24a4225520fd5a609bb44ef2a38dc7869 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Wed, 7 Aug 2024 15:14:15 +0100 Subject: [PATCH 2/7] Check context file for changes from external editor --- damnit/gui/main_window.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/damnit/gui/main_window.py b/damnit/gui/main_window.py index b55e9115..35a47a9c 100644 --- a/damnit/gui/main_window.py +++ b/damnit/gui/main_window.py @@ -51,6 +51,10 @@ class MainWindow(QtWidgets.QMainWindow): context_dir_changed = QtCore.pyqtSignal(str) save_context_finished = QtCore.pyqtSignal(bool) # True if saved context_saved = QtCore.pyqtSignal() + check_context_file_timer = None + vars_ctx_size_mtime = None + editor_ctx_size_mtime = None + updating_vars = False db = None db_id = None @@ -263,18 +267,49 @@ def autoconfigure(self, path: Path): self.show_default_status_message() self.context_dir_changed.emit(str(path)) self.launch_update_computed_vars() + self.start_watching_context_file() def launch_update_computed_vars(self): # Triggered when we open a proposal & when saving the context file log.debug("Launching subprocess to read variables from context file") + self.updating_vars = True proc = QtCore.QProcess(parent=self) # Show stdout & stderr with the parent process proc.setProcessChannelMode(QtCore.QProcess.ProcessChannelMode.ForwardedChannels) + proc.finished.connect(self.done_update_computed_vars) proc.finished.connect(proc.deleteLater) proc.setWorkingDirectory(str(self.context_dir)) proc.start(sys.executable, ['-m', 'damnit.cli', 'read-context']) proc.closeWriteChannel() + def done_update_computed_vars(self): + self.updating_vars = False + self.vars_ctx_size_mtime = self.get_context_size_mtime() + + def get_context_size_mtime(self): + st = self._context_path.stat() + return st.st_size, st.st_mtime + + def poll_context_file(self): + size_mtime = self.get_context_size_mtime() + if (self.vars_ctx_size_mtime != size_mtime) and not self.updating_vars: + log.info("Context file changed, updating computed variables") + self.launch_update_computed_vars() + + if (self.editor_ctx_size_mtime != size_mtime) and self._context_is_saved: + log.info("Context file changed, reloading editor") + self.reload_context() + + def start_watching_context_file(self): + if self.check_context_file_timer is not None: + self.check_context_file_timer.stop() + self.check_context_file_timer.deleteLater() + + self.check_context_file_timer = tmr = QtCore.QTimer(self) + tmr.setInterval(30_000) + tmr.timeout.connect(self.poll_context_file) + tmr.start() + def add_variable(self, name, title, variable_type, description="", before=None): n_static_cols = self.table_view.get_static_columns_count() before_pos = n_static_cols + 1 @@ -865,6 +900,7 @@ def mark_context_saved(self): self._tab_widget.tabBar().setTabTextColor(1, QtGui.QColor("black")) self._editor_status_message = str(self._context_path.resolve()) self.on_tab_changed(self._tab_widget.currentIndex()) + self.editor_ctx_size_mtime = self.get_context_size_mtime() def save_value(self, prop, run, name, value): if self.db is None: From f9a455c399c05e2ba23a12cde64b0d5d434a7f65 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Wed, 23 Oct 2024 15:40:31 +0100 Subject: [PATCH 3/7] Stop file polling timer on window close --- damnit/gui/main_window.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/damnit/gui/main_window.py b/damnit/gui/main_window.py index 35a47a9c..1d798857 100644 --- a/damnit/gui/main_window.py +++ b/damnit/gui/main_window.py @@ -127,6 +127,7 @@ def closeEvent(self, event): return self.stop_update_listener_thread() + self.stop_watching_context_file() super().closeEvent(event) def stop_update_listener_thread(self): @@ -301,15 +302,19 @@ def poll_context_file(self): self.reload_context() def start_watching_context_file(self): - if self.check_context_file_timer is not None: - self.check_context_file_timer.stop() - self.check_context_file_timer.deleteLater() + self.stop_watching_context_file() # Only 1 timer at a time self.check_context_file_timer = tmr = QtCore.QTimer(self) tmr.setInterval(30_000) tmr.timeout.connect(self.poll_context_file) tmr.start() + def stop_watching_context_file(self): + if self.check_context_file_timer is not None: + self.check_context_file_timer.stop() + self.check_context_file_timer.deleteLater() + self.check_context_file_timer = None + def add_variable(self, name, title, variable_type, description="", before=None): n_static_cols = self.table_view.get_static_columns_count() before_pos = n_static_cols + 1 From a8c44128c87333c2ce893db0d44df857e99356be Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Wed, 23 Oct 2024 15:42:12 +0100 Subject: [PATCH 4/7] Emit context_saved only on successful save --- damnit/gui/main_window.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/damnit/gui/main_window.py b/damnit/gui/main_window.py index 1d798857..e3ae4a5d 100644 --- a/damnit/gui/main_window.py +++ b/damnit/gui/main_window.py @@ -854,9 +854,9 @@ def test_context_result(self, test_result, output, checked_code): if saving := test_result is not ContextTestResult.ERROR: self._context_path.write_text(self._context_code_to_save) self.mark_context_saved() + self.context_saved.emit() self._context_code_to_save = None self.save_context_finished.emit(saving) - self.context_saved.emit() if test_result == ContextTestResult.ERROR: self.set_error_widget_text(output) From 017f1732e444ee74a7f371330b8bd5b5a5ef397b Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Wed, 23 Oct 2024 16:50:48 +0100 Subject: [PATCH 5/7] Be more careful about when we get metadata of context file to detect changes --- damnit/gui/main_window.py | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/damnit/gui/main_window.py b/damnit/gui/main_window.py index e3ae4a5d..68810ac6 100644 --- a/damnit/gui/main_window.py +++ b/damnit/gui/main_window.py @@ -270,10 +270,14 @@ def autoconfigure(self, path: Path): self.launch_update_computed_vars() self.start_watching_context_file() - def launch_update_computed_vars(self): + def launch_update_computed_vars(self, ctx_size_mtime=None): # Triggered when we open a proposal & when saving the context file log.debug("Launching subprocess to read variables from context file") self.updating_vars = True + # Store the size & mtime before processing the file: better to capture + # this just before a change and process the same version twice than + # just after & potentially miss a change. + self.vars_ctx_size_mtime = ctx_size_mtime or self.get_context_size_mtime() proc = QtCore.QProcess(parent=self) # Show stdout & stderr with the parent process proc.setProcessChannelMode(QtCore.QProcess.ProcessChannelMode.ForwardedChannels) @@ -285,7 +289,6 @@ def launch_update_computed_vars(self): def done_update_computed_vars(self): self.updating_vars = False - self.vars_ctx_size_mtime = self.get_context_size_mtime() def get_context_size_mtime(self): st = self._context_path.stat() @@ -295,7 +298,7 @@ def poll_context_file(self): size_mtime = self.get_context_size_mtime() if (self.vars_ctx_size_mtime != size_mtime) and not self.updating_vars: log.info("Context file changed, updating computed variables") - self.launch_update_computed_vars() + self.launch_update_computed_vars(size_mtime) if (self.editor_ctx_size_mtime != size_mtime) and self._context_is_saved: log.info("Context file changed, reloading editor") @@ -836,13 +839,31 @@ def on_context_changed(self): self.on_tab_changed(self._tab_widget.currentIndex()) self._context_is_saved = False + def _ctx_contents_size_mtime(self): + """Get the contents of the context file, plus its size & mtime""" + # There's no way to do this atomically, so we stat the file before & + # after reading and check that the results match. + size_mtime_before = self.get_context_size_mtime() + for _ in range(20): + contents = self._context_path.read_text() + size_mtime_after = self.get_context_size_mtime() + if size_mtime_after == size_mtime_before: + return contents, size_mtime_after + + size_mtime_before = size_mtime_after + + raise RuntimeError( + "Could not get consistent filesystem metadata for context file" + ) + def reload_context(self): if not self._context_path.is_file(): self.show_status_message("No context.py file found") return - self._editor.setText(self._context_path.read_text()) + contents, size_mtime = self._ctx_contents_size_mtime() + self._editor.setText(contents) self.test_context() - self.mark_context_saved() + self.mark_context_saved(size_mtime) def test_context(self): self.set_error_icon('wait') @@ -898,14 +919,14 @@ def save_context(self): self.test_context() # If the check passes, .test_context_result() saves the file - def mark_context_saved(self): + def mark_context_saved(self, ctx_size_mtime=None): self._context_is_saved = True self._tabbar_style.enable_bold = False self._tab_widget.setTabText(1, "Context file") self._tab_widget.tabBar().setTabTextColor(1, QtGui.QColor("black")) self._editor_status_message = str(self._context_path.resolve()) self.on_tab_changed(self._tab_widget.currentIndex()) - self.editor_ctx_size_mtime = self.get_context_size_mtime() + self.editor_ctx_size_mtime = ctx_size_mtime or self.get_context_size_mtime() def save_value(self, prop, run, name, value): if self.db is None: From 0cce96235d109ed9fa9b494086781d6095707b4f Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 24 Oct 2024 11:22:56 +0100 Subject: [PATCH 6/7] Don't need to track when variables are being updated --- damnit/gui/main_window.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/damnit/gui/main_window.py b/damnit/gui/main_window.py index 68810ac6..534b8454 100644 --- a/damnit/gui/main_window.py +++ b/damnit/gui/main_window.py @@ -54,7 +54,6 @@ class MainWindow(QtWidgets.QMainWindow): check_context_file_timer = None vars_ctx_size_mtime = None editor_ctx_size_mtime = None - updating_vars = False db = None db_id = None @@ -273,7 +272,6 @@ def autoconfigure(self, path: Path): def launch_update_computed_vars(self, ctx_size_mtime=None): # Triggered when we open a proposal & when saving the context file log.debug("Launching subprocess to read variables from context file") - self.updating_vars = True # Store the size & mtime before processing the file: better to capture # this just before a change and process the same version twice than # just after & potentially miss a change. @@ -281,14 +279,11 @@ def launch_update_computed_vars(self, ctx_size_mtime=None): proc = QtCore.QProcess(parent=self) # Show stdout & stderr with the parent process proc.setProcessChannelMode(QtCore.QProcess.ProcessChannelMode.ForwardedChannels) - proc.finished.connect(self.done_update_computed_vars) proc.finished.connect(proc.deleteLater) proc.setWorkingDirectory(str(self.context_dir)) proc.start(sys.executable, ['-m', 'damnit.cli', 'read-context']) proc.closeWriteChannel() - - def done_update_computed_vars(self): - self.updating_vars = False + # The subprocess will send updates for any changes: see .handle_update() def get_context_size_mtime(self): st = self._context_path.stat() @@ -296,7 +291,7 @@ def get_context_size_mtime(self): def poll_context_file(self): size_mtime = self.get_context_size_mtime() - if (self.vars_ctx_size_mtime != size_mtime) and not self.updating_vars: + if self.vars_ctx_size_mtime != size_mtime: log.info("Context file changed, updating computed variables") self.launch_update_computed_vars(size_mtime) From 476aecd2715ddf2ad48243fd4efeffb89a93af9c Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 24 Oct 2024 11:54:30 +0100 Subject: [PATCH 7/7] Drop context_saved signal --- damnit/gui/main_window.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/damnit/gui/main_window.py b/damnit/gui/main_window.py index 534b8454..11da395c 100644 --- a/damnit/gui/main_window.py +++ b/damnit/gui/main_window.py @@ -50,7 +50,6 @@ class MainWindow(QtWidgets.QMainWindow): context_dir_changed = QtCore.pyqtSignal(str) save_context_finished = QtCore.pyqtSignal(bool) # True if saved - context_saved = QtCore.pyqtSignal() check_context_file_timer = None vars_ctx_size_mtime = None editor_ctx_size_mtime = None @@ -92,7 +91,7 @@ def __init__(self, context_dir: Path = None, connect_to_kafka: bool = True): self._tab_widget.setEnabled(False) self.setCentralWidget(self._tab_widget) - self.context_saved.connect(self.launch_update_computed_vars) + self.save_context_finished.connect(self._save_context_finished) self.table = None @@ -269,6 +268,10 @@ def autoconfigure(self, path: Path): self.launch_update_computed_vars() self.start_watching_context_file() + def _save_context_finished(self, saved): + if saved: + self.launch_update_computed_vars() + def launch_update_computed_vars(self, ctx_size_mtime=None): # Triggered when we open a proposal & when saving the context file log.debug("Launching subprocess to read variables from context file") @@ -870,7 +873,6 @@ def test_context_result(self, test_result, output, checked_code): if saving := test_result is not ContextTestResult.ERROR: self._context_path.write_text(self._context_code_to_save) self.mark_context_saved() - self.context_saved.emit() self._context_code_to_save = None self.save_context_finished.emit(saving)