From e45ea74640378452fdde72f4932e38ccce228310 Mon Sep 17 00:00:00 2001 From: Caio Fonseca Date: Fri, 4 Sep 2026 13:26:14 +0100 Subject: [PATCH 1/3] fix: handle empty repo selections in the cache facade Callbacks fire on initial page load with repo-choices still [], and psycopg2 renders an empty sequence as "()", so "WHERE repo_id in ()" errored on every fresh session. get_uncached returns [] early. retrieve_from_cache sends a NULL sentinel rather than returning early, since a bare DataFrame has no columns and twelve call sites preprocess on columns before testing df.empty. get_default_repo_with_data guards separately -- it indexes repo_ids[0]. --- 8Knot/cache_manager/cache_facade.py | 15 ++++++++++++++- 8Knot/pages/utils/job_utils.py | 6 ++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/8Knot/cache_manager/cache_facade.py b/8Knot/cache_manager/cache_facade.py index 955b231d..dbbc01de 100644 --- a/8Knot/cache_manager/cache_facade.py +++ b/8Knot/cache_manager/cache_facade.py @@ -124,6 +124,14 @@ def get_uncached(func_name: str, repolist: list[int]) -> list[int]: # or None Returns a list of repos that AREN'T resident in cache. """ + if not repolist: + # nothing requested means nothing missing. guarded here rather than at + # each call site because psycopg2 renders an empty sequence as "in ()", + # which postgres rejects outright -- and every visualization callback + # polls this function on initial page load, when the repo-choices + # store still holds its default []. + return [] + with pg.connect(cache_cx_string) as cache_conn: with cache_conn.cursor() as cache_cur: composed_query = pg_sql.SQL( @@ -224,7 +232,12 @@ def retrieve_from_cache( """.format( tablename=tablename ), - (tuple(repolist),), + # psycopg2 renders an empty sequence as "()", which postgres + # rejects. NULL matches nothing, so an empty repolist returns + # zero rows while cursor.description still yields the table's + # columns -- callers preprocess on those columns before they + # test df.empty, so a 0x0 frame is not a safe substitute. + (tuple(repolist) or (None,),), ) logging.warning(f"{tablename} - LOADING DATA FROM CACHE") diff --git a/8Knot/pages/utils/job_utils.py b/8Knot/pages/utils/job_utils.py index 7219b9a0..3d95555b 100644 --- a/8Knot/pages/utils/job_utils.py +++ b/8Knot/pages/utils/job_utils.py @@ -54,6 +54,12 @@ def get_default_repo_with_data(repo_ids, cache_tablename): str: The first repo_id (as string) that has cached data, or repo_ids[0] as fallback """ + if not repo_ids: + # no selection to default to. this runs in the web process on initial + # page load, when the repo-choices store still holds its default [], + # and every return below indexes repo_ids[0]. + return None + df = cf.retrieve_from_cache(tablename=cache_tablename, repolist=repo_ids) if df.empty: From adac4f832d96c4ec3955a74b90324f8cbe868916 Mon Sep 17 00:00:00 2001 From: Caio Fonseca Date: Fri, 4 Sep 2026 13:26:27 +0100 Subject: [PATCH 2/3] fix: preserve error causes and stop bookkeeping already-cached repos run_query's bare except also caught KeyboardInterrupt/SystemExit and dropped the cause, so a timeout, a bad column and a dropped connection were indistinguishable. _bots.py logged the KeyError then fell through to an unbound dbm, raising NameError instead. caching_wrapper recorded bookkeeping for the full repolist rather than the repos it actually cached, adding a duplicate row per already-resident repo to the table the poll loops query twice a second. The stale name came from rebinding uncached_repos to the query vars; those are now separate. --- 8Knot/_bots.py | 13 +++++++++---- 8Knot/cache_manager/cache_facade.py | 12 ++++++++---- 8Knot/db_manager/augur_manager.py | 8 ++++++-- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/8Knot/_bots.py b/8Knot/_bots.py index eeeaaaa5..3b99a72c 100644 --- a/8Knot/_bots.py +++ b/8Knot/_bots.py @@ -15,14 +15,19 @@ def get_bots_list(): try: dbm = AugurManager() - engine = dbm.get_engine() + dbm.get_engine() except KeyError: - # noack, data wasn't successfully set. + # defensive- currently unreachable, since app.py:38-44 constructs an + # AugurManager over the same credentials and sys.exit(1)s first. + # falling through from here left dbm unbound, so the caller would have + # seen a NameError rather than this. logging.error("BOT_DATA_QUERY - INCOMPLETE ENVIRONMENT") + raise except SQLAlchemyError: + # runs at import in the web process, so there's no Celery retry to + # fall back on- fail the boot rather than serve unfiltered bot data. logging.error("BOT_DATA_QUERY - COULDN'T CONNECT TO DB") - # allow retry via Celery rules. - raise SQLAlchemyError("DBConnect failed") + raise df = dbm.run_query(query_string) # reformat cntrb_id diff --git a/8Knot/cache_manager/cache_facade.py b/8Knot/cache_manager/cache_facade.py index dbbc01de..120865f1 100644 --- a/8Knot/cache_manager/cache_facade.py +++ b/8Knot/cache_manager/cache_facade.py @@ -189,17 +189,21 @@ def caching_wrapper(func_name: str, query: str, repolist: list[int], n_repolist_ logging.warning(f"{func_name} COLLECTION - CACHING {len(uncached_repos)} NEW REPOS") # inject the repolist multiple times because the SQL uses it more - # than once and the wildcard %s are ordered. - uncached_repos: tuple[tuple] = tuple([tuple(uncached_repos) for _ in range(n_repolist_uses)]) + # than once and the wildcard %s are ordered. kept under its own name + # so uncached_repos stays the list of repos this call is filling. + query_vars: tuple[tuple] = tuple([tuple(uncached_repos) for _ in range(n_repolist_uses)]) # STEP 2: Query for those repos logging.warning(f"{func_name} COLLECTION - EXECUTING CACHING QUERY") cache_query_results( db_connection_string=db_cx_string, query=query, - vars=uncached_repos, + vars=query_vars, target_table=func_name, - bookkeeping_data=tuple({"cache_func": func_name, "repo_id": r} for r in repolist), + # only the repos actually cached here. recording the full repolist + # re-inserted a row for every repo that was already resident, and + # cache_bookkeeping has no unique constraint to absorb the repeats. + bookkeeping_data=tuple({"cache_func": func_name, "repo_id": r} for r in uncached_repos), ) except Exception as e: logging.critical(f"{func_name}_POSTGRES ERROR: {e}") diff --git a/8Knot/db_manager/augur_manager.py b/8Knot/db_manager/augur_manager.py index f0e11c44..440a182a 100644 --- a/8Knot/db_manager/augur_manager.py +++ b/8Knot/db_manager/augur_manager.py @@ -153,8 +153,12 @@ def run_query(self, query_string: str) -> pd.DataFrame: try: with self.engine.connect() as conn: result_df = pd.read_sql(query, con=conn) - except: - raise Exception("DB Read Failure") + except Exception as e: + # a bare 'except' here also caught KeyboardInterrupt and SystemExit, + # and dropped the cause -- so a statement timeout, a bad column name + # and a dropped connection were indistinguishable in the logs. + logging.exception("AUGUR: DB read failed") + raise Exception("DB Read Failure") from e result_df = result_df.reset_index() result_df.drop("index", axis=1, inplace=True) From 584a066e3c0f964093f06bad8b07b43e63b9339a Mon Sep 17 00:00:00 2001 From: Caio Fonseca Date: Fri, 4 Sep 2026 14:20:28 +0100 Subject: [PATCH 3/3] fix: guard empty repo-dependent callbacks Signed-off-by: Caio Fonseca --- 8Knot/pages/codebase/visualizations/cntrb_file_heatmap.py | 6 ++++++ .../codebase/visualizations/contribution_file_heatmap.py | 6 ++++++ .../pages/codebase/visualizations/reviewer_file_heatmap.py | 6 ++++++ 8Knot/pages/repo_overview/repo_overview.py | 2 +- 8Knot/pages/repo_overview/visualizations/ossf_scorecard.py | 6 ++++-- .../pages/repo_overview/visualizations/repo_general_info.py | 6 ++++-- 8Knot/pages/utils/job_utils.py | 2 +- 7 files changed, 28 insertions(+), 6 deletions(-) diff --git a/8Knot/pages/codebase/visualizations/cntrb_file_heatmap.py b/8Knot/pages/codebase/visualizations/cntrb_file_heatmap.py index 606899c9..384348f0 100644 --- a/8Knot/pages/codebase/visualizations/cntrb_file_heatmap.py +++ b/8Knot/pages/codebase/visualizations/cntrb_file_heatmap.py @@ -110,6 +110,9 @@ def repo_dropdown(repo_ids): background=True, ) def directory_dropdown(repo_id): + if repo_id is None: + return [], None + # Convert to int since Mantine dropdown returns strings repo_id = int(repo_id) @@ -190,6 +193,9 @@ def cntrb_file_heatmap_graph(searchbar_repos, repo_id, directory, bot_switch): start = time.perf_counter() logging.warning(f"{VIZ_ID}- START") + if repo_id is None: + return nodata_graph + # Convert to int since Mantine dropdown returns strings repo_id = int(repo_id) diff --git a/8Knot/pages/codebase/visualizations/contribution_file_heatmap.py b/8Knot/pages/codebase/visualizations/contribution_file_heatmap.py index aa700281..c14d2df5 100644 --- a/8Knot/pages/codebase/visualizations/contribution_file_heatmap.py +++ b/8Knot/pages/codebase/visualizations/contribution_file_heatmap.py @@ -125,6 +125,9 @@ def repo_dropdown(repo_ids): background=True, ) def directory_dropdown(repo_id): + if repo_id is None: + return [], None + # Convert to int since Mantine dropdown returns strings repo_id = int(repo_id) @@ -200,6 +203,9 @@ def cntrb_file_heatmap_graph(repo_id, directory, graph_view): start = time.perf_counter() logging.warning(f"{VIZ_ID}- START") + if repo_id is None: + return nodata_graph + # Convert to int since Mantine dropdown returns strings repo_id = int(repo_id) diff --git a/8Knot/pages/codebase/visualizations/reviewer_file_heatmap.py b/8Knot/pages/codebase/visualizations/reviewer_file_heatmap.py index 956fbff6..5f6cc5b0 100644 --- a/8Knot/pages/codebase/visualizations/reviewer_file_heatmap.py +++ b/8Knot/pages/codebase/visualizations/reviewer_file_heatmap.py @@ -110,6 +110,9 @@ def repo_dropdown(repo_ids): background=True, ) def directory_dropdown(repo_id): + if repo_id is None: + return [], None + # Convert to int since Mantine dropdown returns strings repo_id = int(repo_id) @@ -190,6 +193,9 @@ def reviewer_file_heatmap_graph(searchbar_repos, repo_id, directory, bot_switch) start = time.perf_counter() logging.warning(f"{VIZ_ID}- START") + if repo_id is None: + return nodata_graph + # Convert to int since Mantine dropdown returns strings repo_id = int(repo_id) diff --git a/8Knot/pages/repo_overview/repo_overview.py b/8Knot/pages/repo_overview/repo_overview.py index 12ea99fc..298dbd7b 100644 --- a/8Knot/pages/repo_overview/repo_overview.py +++ b/8Knot/pages/repo_overview/repo_overview.py @@ -78,4 +78,4 @@ def repo_dropdown(repo_ids): for repo_id in repo_ids: entry = {"value": str(repo_id), "label": augur.repo_id_to_git(int(repo_id))} data_array.append(entry) - return data_array, str(repo_ids[0]) + return data_array, str(repo_ids[0]) if repo_ids else None diff --git a/8Knot/pages/repo_overview/visualizations/ossf_scorecard.py b/8Knot/pages/repo_overview/visualizations/ossf_scorecard.py index 30a5ec07..5a7baa4a 100644 --- a/8Knot/pages/repo_overview/visualizations/ossf_scorecard.py +++ b/8Knot/pages/repo_overview/visualizations/ossf_scorecard.py @@ -99,8 +99,10 @@ def toggle_popover(n, is_open): ) def ossf_scorecard(repo: str): - if repo is not None: - repo = int(repo) + if repo is None: + return dbc.Table.from_dataframe(pd.DataFrame(), striped=True, bordered=True, hover=True), dbc.Label("No data") + + repo = int(repo) # wait for data to asynchronously download and become available. while not_cached := cf.get_uncached(func_name=osq.__name__, repolist=[repo]): diff --git a/8Knot/pages/repo_overview/visualizations/repo_general_info.py b/8Knot/pages/repo_overview/visualizations/repo_general_info.py index a0924d56..de0a5f5b 100644 --- a/8Knot/pages/repo_overview/visualizations/repo_general_info.py +++ b/8Knot/pages/repo_overview/visualizations/repo_general_info.py @@ -80,8 +80,10 @@ def toggle_popover(n, is_open): ) def repo_general_info(repo): - if repo is not None: - repo = int(repo) + if repo is None: + return dbc.Table.from_dataframe(pd.DataFrame(), striped=True, bordered=True, hover=True), dbc.Label("No data") + + repo = int(repo) logging.warning(f"{VIZ_ID} - START") start = time.perf_counter() diff --git a/8Knot/pages/utils/job_utils.py b/8Knot/pages/utils/job_utils.py index 3d95555b..409ce633 100644 --- a/8Knot/pages/utils/job_utils.py +++ b/8Knot/pages/utils/job_utils.py @@ -51,7 +51,7 @@ def get_default_repo_with_data(repo_ids, cache_tablename): cache_tablename: Name of the cache table to query Returns: - str: The first repo_id (as string) that has cached data, or repo_ids[0] as fallback + str | None: The first repo_id with cached data, repo_ids[0] as fallback, or None when no repos are selected """ if not repo_ids: