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

Fix for super() usage on "old style class" ZipFile #546

Merged
merged 1 commit into from
Sep 6, 2018

Conversation

cbrinker
Copy link
Contributor

I was experiencing an error of "TypeError: must be type, not classobj"
on the line using super()._extract_member() below.
This error appears to be due to the zipfile.ZipFile class never inheriting from
object which is a requirement since py2.2 (2001).

Since the use of super() in PermPreservingZipFile() depends on
object being somewhere in the inheritance tree, I am including it in
the class definition of PermPreservingZipFile(). My testing of this patch
locally has proven to solve the "TypeError" I was seeing.

I was experienceing an error of "TypeError: must be type, not classobj"
on the line using `super()._extract_member()` below.
This error appears to be due to the `zipfile.ZipFile` class never inheriting from
`object` which is a requirement since py2.2 (2001).

Since the use of `super()` in `PermPreservingZipFile()` depends on
`object` being somewhere in the inheritance tree, I am including it in
the class definition of `PermPreservingZipFile()`. My testing of this patch
locally has proven to solve the "TypeError" I was seeing.
@jsirois
Copy link
Member

jsirois commented Aug 31, 2018

CI is continuing to fail on 1 shard despite me re-running it a few times. It does not likely to be related to the change, but one thing that seems a bit off is multiple-inheriting from object instead of using old-style super to go along with the old-style baseclass. IE:

result = PermPreservingZipFile._extract_member(self, member, targetpath, pwd)

@jsirois
Copy link
Member

jsirois commented Sep 6, 2018

Alrighty - got this green with another shard restart :/ Thanks @cbrinker !

@jsirois jsirois merged commit 9bba75d into pex-tool:master Sep 6, 2018
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.

2 participants