fix: improve NLTK resource handling and logging in GCS document retri… #1395
Conversation
praveshkumar1988
requested review from
Copilot,
karanchellani and
kaustubh-darekar
There was a problem hiding this comment.
Pull Request Overview
This PR improves NLTK resource management and error handling in the GCS document retrieval system. The changes make the code more robust by preventing duplicate NLTK data path entries, providing clearer logging messages, and handling potential None values from PDF text extraction.
Key Changes:
- Refactored NLTK resource initialization to avoid duplicate path entries and streamline resource downloading
- Enhanced logging to specify download locations and indicate when resources are missing
- Added null-safety guard for PDF text extraction
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if "/usr/local/nltk_data" not in nltk.data.path: | ||
| nltk.data.path.append("/usr/local/nltk_data") |
There was a problem hiding this comment.
This duplicate check for '/usr/local/nltk_data' is redundant. The path is already checked and added in lines 51-53 before the resource download loop. Remove this redundant check to improve code clarity.
| if "/usr/local/nltk_data" not in nltk.data.path: | |
| nltk.data.path.append("/usr/local/nltk_data") |
| nltk.download(resource, download_dir=os.path.expanduser("~/.nltk_data")) | ||
| logging.info(f"NLTK resource '{res}' not found; downloading to /usr/local/nltk_data") | ||
| nltk.download(res, download_dir="/usr/local/nltk_data") | ||
|
|
There was a problem hiding this comment.
Trailing whitespace on this blank line should be removed for consistency with project style.
…document retrieval
…eval