Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gzip support #171

Merged
merged 9 commits into from
Jun 4, 2024
Merged

Add gzip support #171

merged 9 commits into from
Jun 4, 2024

Conversation

ecalifornica
Copy link
Contributor

*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 gzip and tempfile, 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

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 Show resolved Hide resolved
@pablogsal
Copy link
Member

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>
@pablogsal
Copy link
Member

pablogsal commented May 13, 2024

Great job! We are almost there

You need a news entry - an .rst file in the news folder (the other error in the Lint step you can ignore as we are fixing this in other PR).

Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
@ecalifornica
Copy link
Contributor Author

Is this the correct format for the news entry?

Signed-off-by: Robert Queenin <2177841+ecalifornica@users.noreply.github.com>
@ecalifornica
Copy link
Contributor Author

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.

@pablogsal pablogsal merged commit b163f2d into bloomberg:main Jun 4, 2024
21 checks passed
@pablogsal
Copy link
Member

Sorry this took too long to merge @ecalifornica and congrats on the merge!

Thanks a lot for being so patient with this and for following our (slow) feedback!

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.

None yet

2 participants