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

be open-minded about byte and dict headers #19

Merged
merged 5 commits into from
Jul 1, 2017
Merged

be open-minded about byte and dict headers #19

merged 5 commits into from
Jul 1, 2017

Conversation

wumpus
Copy link
Collaborator

@wumpus wumpus commented May 12, 2017

This is as discussed in #16

I kind of punted on converting bytes/str/whatever for Python 2 because I don't really understand it. It still does dict -> list for python2.

If the user happens to have multidict installed (part of aiohttp), then the multidict tests will run. This is optional because warcio strives to avoid external dependencies. If there's a more-correct way to get that into the CI, I'd love to know it.

@coveralls
Copy link

coveralls commented May 12, 2017

Coverage Status

Coverage decreased (-0.1%) to 99.903% when pulling 5485134 on cocrawler:header-frobbing2 into 173aa90 on webrecorder:develop.

@wumpus
Copy link
Collaborator Author

wumpus commented May 12, 2017

I don't understand the coveralls fail. The marked line is executed by both python2 and python3.

@wumpus
Copy link
Collaborator Author

wumpus commented May 12, 2017

shit. missing a file.

@coveralls
Copy link

coveralls commented May 12, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling f8ccd14 on cocrawler:header-frobbing2 into 173aa90 on webrecorder:develop.

@coveralls
Copy link

coveralls commented May 12, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling ff079b2 on cocrawler:header-frobbing2 into 173aa90 on webrecorder:develop.

@wumpus
Copy link
Collaborator Author

wumpus commented May 12, 2017

Oy. Hit the "squash merge" button :-)

@coveralls
Copy link

coveralls commented May 14, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 72ef825 on cocrawler:header-frobbing2 into 173aa90 on webrecorder:develop.

@ikreymer ikreymer merged commit 7e62180 into webrecorder:develop Jul 1, 2017
@ikreymer
Copy link
Member

ikreymer commented Jul 1, 2017

Thanks for this, and sorry for the delay.

I wonder if its worth optimizing for headers already being in str format:

if h and type(h[0][0]) == str and type(h[0][1]) == str:
   return h

I suppose its unlikely that there'd be a mix of str and byte headers, but not sure if this would really make any difference

@wumpus wumpus deleted the header-frobbing2 branch July 1, 2017 16:26
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.

3 participants