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 955b231d..120865f1 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( @@ -181,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}") @@ -224,7 +236,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/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) 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 7219b9a0..409ce633 100644 --- a/8Knot/pages/utils/job_utils.py +++ b/8Knot/pages/utils/job_utils.py @@ -51,9 +51,15 @@ 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: + # 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: