Skip to content

fix: replace bare except clauses with except Exception in ingest.py#5

Open
harshadkhetpal wants to merge 43 commits into
bibinprathap:restored-mainfrom
harshadkhetpal:fix/bare-except-ingest
Open

fix: replace bare except clauses with except Exception in ingest.py#5
harshadkhetpal wants to merge 43 commits into
bibinprathap:restored-mainfrom
harshadkhetpal:fix/bare-except-ingest

Conversation

@harshadkhetpal
Copy link
Copy Markdown

Summary

Replace 5 bare except: clauses with except Exception: in graphrag-ollama-config/ingest.py (lines 183, 236, 561, 642, 696). Bare excepts catch all exceptions including SystemExit, KeyboardInterrupt, and GeneratorExit, which should generally propagate.

Change:

# Before
except:
    pass

# After
except Exception:
    pass

Testing

No behavior change for normal exceptions — style/correctness fix per PEP 8.

bibinprathap and others added 30 commits February 3, 2026 22:27
Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from bare except: to except Exception: is correct and prevents accidentally swallowing KeyboardInterrupt, SystemExit, and GeneratorExit. That said, several of these handlers could be narrowed further to improve debuggability — for example, the os.remove(log_path) call in trigger_graphrag_index_with_progress around line 642 realistically only needs to handle FileNotFoundError or PermissionError, and the pd.read_parquet calls (lines 561, 720, 838) could catch FileNotFoundError or pyarrow/fastparquet exceptions specifically. All of these except Exception: pass blocks silently discard errors with no logging, which makes diagnosing failures in production difficult — even a logging.debug(exc_info=True) would significantly improve observability. The get_last_run_id function is a good candidate for tighter handling since a failure there likely means the log file is malformed or missing, which are distinguishable and actionable conditions worth surfacing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants