Improved error handling and readability of extractor #826
Conversation
| @staticmethod | ||
| async def extract_file_tar(filename, extraction_path): | ||
| """ Extract tar files """ | ||
| try: | ||
| with ErrorHandler(mode=ErrorMode.Ignore) as e: | ||
| await aio_unpack_archive(filename, extraction_path) | ||
| return 0 | ||
| except Exception: | ||
| return 1 | ||
| return e.exit_code |
There was a problem hiding this comment.
This is static method because we don't need class attribute here. We are just ignoring error
| with ErrorHandler(mode=self.error_mode, logger=self.logger): | ||
| raise Exception( | ||
| "'rpm2cpio' and 'cpio' are required to extract rpm files" | ||
| ) |
There was a problem hiding this comment.
Here we are raising error so, we want to log that.
| class Extractor(BaseExtractor): | ||
| def Extractor(*args, **kwargs): | ||
| """Provides a context which extraction is done in""" | ||
|
|
||
| def __call__(self, *args, **kwargs): | ||
| return TempDirExtractorContext(*args, **kwargs) | ||
| return TempDirExtractorContext(*args, **kwargs) |
There was a problem hiding this comment.
Create function instead of callable class for better readability. Ex:
Extractor()(error_mode=mode) will now become Extractor(error_mode=mode)
Niraj-Kamdar
changed the title
Niraj-Kamdar
changed the title
Codecov Report
@@ Coverage Diff @@
## master #826 +/- ##
==========================================
+ Coverage 83.16% 88.58% +5.42%
==========================================
Files 150 150
Lines 2542 2541 -1
Branches 279 278 -1
==========================================
+ Hits 2114 2251 +137
+ Misses 359 227 -132
+ Partials 69 63 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This looks good to me, but I'll leave it to @pdxjohnny to approve since he wrote the original code and might have more precise feedback.
|
K, it's been 3 days, I'm going to assume @pdxjohnny doesn't have time to review this. Looks good enough to me so let's get it merged! |
No description provided.