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

reintroduced inputfile.seek(0) to utils.uncompress_file #503

Merged
merged 5 commits into from
Jan 29, 2024

Conversation

truePhilipp
Copy link
Contributor

@truePhilipp truePhilipp commented Jan 27, 2024

Description

This is a very small fix for #292.
I think this issue got introduced way back by #189.
In there line 244 inputfile.seek(0) got removed, which seems to cause this issue.
Simply reintroducing this line makes the uncompressing work again.

Checklist

Please update this checklist as you complete each item:

  • Tests have been developed for bug fixes or new functionality.
  • The changelog has been updated, if necessary.
  • Documentation has been updated, if necessary.
  • GitHub Issues closed by this PR have been linked.

By submitting this pull request I agree that all contributions comply with this project's open source license(s).

Copy link

codecov bot commented Jan 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b67a437) 91.21% compared to head (91eabff) 91.13%.

❗ Current head 91eabff differs from pull request most recent head da41e01. Consider uploading reports for the commit da41e01 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #503      +/-   ##
==========================================
- Coverage   91.21%   91.13%   -0.09%     
==========================================
  Files          19       19              
  Lines         854      857       +3     
  Branches      150      150              
==========================================
+ Hits          779      781       +2     
- Misses         40       41       +1     
  Partials       35       35              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Archmonger Archmonger linked an issue Jan 28, 2024 that may be closed by this pull request
@Archmonger Archmonger marked this pull request as draft January 28, 2024 23:32
@Archmonger
Copy link
Contributor

Can you add a line item on the changelog, and a unit test (if feasible)?

@truePhilipp
Copy link
Contributor Author

@Archmonger yes, sure. The changelog is docs/changelog.rst right? The newest main point is 4.0.0b0 (2021-12-19) should I just add it there?

@Archmonger
Copy link
Contributor

This change would belong in an Unreleased section until a formal release is made.

On a side note, looks like there was a bit of forgetfulness of bumping the changelog from @johnthagen and I.

I will retroactively add v4.0.2 and v4.1.0 to the changelog in a separate PR.

@Archmonger
Copy link
Contributor

Changelog PR has been created: #504

@truePhilipp
Copy link
Contributor Author

Ok, i added a test and the edited the changelog.
I have noticed that the test DbRestoreCommandTest.test_compressed does not actually check if the restore succeeded…
I don‘t know where to report that, so i‘ll just do it here. It could be fixed by adding two lines, should I do that in this merge request as well, or should i open another one for that, since it has nothing to do with this?

@Archmonger
Copy link
Contributor

That's pretty minor so feel free to fold it into this PR.

@Archmonger Archmonger marked this pull request as ready for review January 29, 2024 22:38
Copy link
Contributor

@Archmonger Archmonger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR can be merged after resolving merge conflicts.

@Archmonger Archmonger merged commit a8deb81 into jazzband:master Jan 29, 2024
7 checks passed
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.

Restore of database backup from S3 storage fails
2 participants