Add gzip support #171
Add gzip support #171
Conversation
Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
src/pystack/__main__.py
Outdated
| @@ -319,6 +321,19 @@ def process_core(parser: argparse.ArgumentParser, args: argparse.Namespace) -> N | |||
| if not corefile.exists(): | |||
| parser.error(f"Core {corefile} does not exist") | |||
|
|
|||
| temp_file = None | |||
| if corefile.suffix == ".gz": | |||
There was a problem hiding this comment.
Maybe we can improve this to not rely on the filename by checking the gz magic number: 1f 8b from the header
There was a problem hiding this comment.
In making this change I hit a small snag: other tests mock the pathlib.Path().exists() check, causing them to fail when attempting to open the non-existent corefile in order to check the header. What do you think the correct approach would be here?
- Update those tests to expect the read error?
- Create a corefile fixture?
- Something else?
There was a problem hiding this comment.
Move this code to a function and mock/patch it in those tests to return None or whatever indicates that this operation doesn't make sense
There was a problem hiding this comment.
Good call, I should've done this to begin with. Thanks!
|
Thanks a lot for the PR! I will try to review it this week :) |
Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
|
Great job! We are almost there You need a news entry - an |
|
Is this the correct format for the news entry? |
Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
|
Thanks for running the checks yesterday @godlygeek. I've picked up your fix for the Alpine test failure and removed the code that was causing the Python3.7 failure. Hopefully those changes should cause this PR to go green. |
*Issue number of the reported bug or feature request: #168 *
Describe your changes
Add check for core dump file ending in
.gz. If so, decompress to temporary file and run analyzer on temporary file.Testing performed
Added a unit test following the pattern of current tests. It mocks calls to
gzipandtempfile, asserts that the analyzer was called with the correct path. It does not test for any failure modes, and really only tests the mocks. Happy to add more tests if requested.Additional context