Pooled and buffered gzip implementation #5722
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5722 +/- ##
==========================================
+ Coverage 37.73% 37.91% +0.17%
==========================================
Files 327 328 +1
Lines 47895 48084 +189
==========================================
+ Hits 18074 18229 +155
- Misses 27222 27238 +16
- Partials 2599 2617 +18
Continue to review full report at Codecov.
|
bkcsoft
added
the
lgtm/need 2
|
@zeripath could you add some integration tests? |
|
OK I've added a simple test, and an integration test that will get from the LFS store objects which are expected to be compressed and not compressed. The integration test basically requires running the suite with gzip enabled and without gzip enabled - which is probably reasonable given we missed a problem with gzip although it does make our test suite very long. I am happy to drop the integration test doubling if you think that the basic test would be sufficient. Actually I've just made the sqlite integration test run with gzip enabled. It seemed the most simple way. |
|
Thinking on, although that trick with Enable_gzip and then rerun integration tests doesn't actually fail it is remarkably inefficient in that only my integration test actually runs differently. @lunny is the simple gzip_test sufficient or do you want integration tests. I can make it so that our integration tests actually deal with gzip'd content in general if you like. I'm actually wrong here, unless we turn it off Now I just need to work out what postgres is complaining about. |
|
Ah it looks like my integration test is hitting go-testfixtures/testfixtures#39 |
zeripath
force-pushed
the
issue-5182-better-gzip-encoder-management
branch
from
6d70b15
to
e61c98e
Compare
|
I've just rebased and squashed up some of the results. |
The previous code made it possible for a race condition to occur whereby a LFSMetaObject could be checked into the database twice. We should check if the LFSMetaObject is within the database and insert it if not in one transaction.
The integration tests are being affected by go-testfixtures/testfixtures#39 if we set the primary key high enough, keep a count of this and remove at the end of each test we shouldn't be affected by this.
zeripath
force-pushed
the
issue-5182-better-gzip-encoder-management
branch
from
75bffac
to
20aef08
Compare
bkcsoft
added
lgtm/need 1
|
@lunny need your review |
bkcsoft
added
lgtm/done
* Pooled and buffered gzip implementation * Add test for gzip * Add integration test * Ensure lfs check within transaction The previous code made it possible for a race condition to occur whereby a LFSMetaObject could be checked into the database twice. We should check if the LFSMetaObject is within the database and insert it if not in one transaction. * Try to avoid primary key problem in postgres The integration tests are being affected by go-testfixtures/testfixtures#39 if we set the primary key high enough, keep a count of this and remove at the end of each test we shouldn't be affected by this.
This PR reimplements the gzip middleware implementation with a pooled and buffered gzip implementation. It more properly handles gzipped content and zipped content.
It should fix #5182 & fix #4853.