Rewind file pointer and be compatible with 3.13.13#84
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #84 +/- ##
==========================================
+ Coverage 80.84% 80.89% +0.04%
==========================================
Files 26 26
Lines 2370 2381 +11
==========================================
+ Hits 1916 1926 +10
- Misses 454 455 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
twiggler
changed the title
| @classmethod | ||
| def frombuf(cls, buf: bytes, encoding: str, errors: str) -> VisorTarInfo: | ||
| obj = super().frombuf(buf, encoding, errors) | ||
| cls._init_visor_attrs(obj, buf) |
There was a problem hiding this comment.
wouldn't it now technically call the _init_visor_attrs twice? as super().frombuf calls cls._frombuf which already initiates _init_visor_attrs?
There was a problem hiding this comment.
Yeah, but _init_visor_attrs is idempotent
| fileobj.seek(0) | ||
| fileobj = BytesIO(fileobj.read()) | ||
| if compressed: | ||
| # GzipFile/LZMAFile are not seekable, but TarFile requires seek/tell. |
There was a problem hiding this comment.
But they are? That wasn't the reason I did the read into BytesIO.
There was a problem hiding this comment.
Ok, what was the reason?
There was a problem hiding this comment.
It's was in the comment you removed. Performance reasons for random reads.
| # The test file has no textPgs/fixUpPgs | ||
| assert all(member.is_visor for member in members.values()) | ||
| assert set(members.keys()) == { | ||
| "test", |
There was a problem hiding this comment.
Reviewer: The directory should also be a member, right?
There was a problem hiding this comment.
I would think it should also be included, and not only the file contents. @Schamper was there a reason it wasn't included?
There was a problem hiding this comment.
Probably oversight because I didn't rewind the file pointer :) should be included.
| fileobj.seek(0) | ||
| fileobj = BytesIO(fileobj.read()) | ||
| if compressed: | ||
| # GzipFile/LZMAFile are not seekable, but TarFile requires seek/tell. |
There was a problem hiding this comment.
Ok, what was the reason?
| @classmethod | ||
| def frombuf(cls, buf: bytes, encoding: str, errors: str) -> VisorTarInfo: | ||
| obj = super().frombuf(buf, encoding, errors) | ||
| cls._init_visor_attrs(obj, buf) |
There was a problem hiding this comment.
Yeah, but _init_visor_attrs is idempotent
3 participants
Two fixes:
Problem:
We did not rewind the file pointer for uncompressed vmtar files, skipping the first entry
Problem:
Python3.13.13 bypasses frombuf and directly calls _frombuf
(Found by nightly monorepo ci)
It seems our current CI is still using 3.13.12