Missing config file errors are not logged to event log in file-extractor#506
Missing config file errors are not logged to event log in file-extractor#506Yaseen-A-Khan wants to merge 1 commit intomasterfrom
Conversation
Summary of ChangesHello @Yaseen-A-Khan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the error handling mechanism within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an important issue where an invalid configuration would cause the extractor process to terminate abruptly. By removing the try...except block in Extractor.__enter__, the InvalidConfigError is now allowed to propagate to the caller. This is a significant improvement in error handling, making the extractor more robust and predictable when used as a library component. The change is accompanied by a well-written unit test that confirms the new behavior. The implementation is clean and effective, and I have no further recommendations.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #506 +/- ##
==========================================
+ Coverage 81.13% 81.45% +0.32%
==========================================
Files 43 43
Lines 4208 4201 -7
==========================================
+ Hits 3414 3422 +8
+ Misses 794 779 -15
🚀 New features to boost your workflow:
|
This PR adds changes as per the DOG-5806 - Missing config file errors are not logged to event log in file-extractor
Problem: In
Extractor.__enter__enter when_initial_load_configraised anInvalidConfigError, the code caught it, printed to stderr, and calledsys.exit(1). This swallowed the exception, killed the process, and gave callers no way to handle the error.Fix:
cognite/extractorutils/base.py -- Removed the try/except block around
_initial_load_config()so InvalidConfigError propagates naturally to the caller. Removed the now-unusedimport sys.tests/tests_unit/test_base.py -- Added test_enter_raises_on_invalid_config that mocks
_initial_load_configto raise InvalidConfigError and asserts it propagates from__enter__.