Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Merge from master (October 30th, 2019) #136

Merged

Conversation

pquentin
Copy link
Member

The tricky part here, as identified by @RatanShreshtha is the chunked commit. Here's my merge commit: 4de9315

I switched to use lower-case headers: b"transfer-encoding: chunked", and sent a request with a body, in order to trigger chunked encoding. I use an iterable body so that it keeps working if/when we start supporting those. See #135 for details. My goal here wasn't to decide what we'll be doing in the future, but just get this merged.

Everything else went quite smoothly. I just had to circumvent mhammond/pywin32#1439, fixed in f331ec4.

lmvlmv and others added 29 commits October 29, 2019 07:19
It reuses a standard mechanism, is less verbose and avoids including
code that is never executed.
Queue.get() accepts any non-negative number, so 0 works. It's also not
arbitrary like 1 was, and the test suite now takes one less second to
run!
It was introduced in urllib3/urllib3#875, but
since urllib3/urllib3#1495 twine is no longer a
dev requirement.
Instead, install it on demand like twine.
The idea is to make sure that we'll get the pytest version we want: the
last that supports Python 2. Using a different version in Python 2 and
Python 3 is possible, but is likely to lead to trouble in the future.
It turns out that without turning this one it's difficult to list
exactly the tests that failed.
The default pytest tracebacks are really verbose: for each function in
the stack they include the whole function until the exception, and the
representation of its parameters.

On the other hand, native tracebacks are way more concise, showing only
two lines for each function and exception chains.

The benefits of switching to native tracebacks:

 * The useful information fits in one screen
 * Python trains us to read those tracebacks from day one
 * We avoid the verbosity of showing the source code: we can already
   see it in GitHub and our editors
 * While we lose the information about parameters, I've never found this
   information to be useful

I've been using this configuration for a few months now and never looked
back!
@codecov
Copy link

codecov bot commented Nov 12, 2019

Codecov Report

Merging #136 into bleach-spike will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           bleach-spike     #136   +/-   ##
=============================================
  Coverage         99.25%   99.25%           
=============================================
  Files                29       29           
  Lines              2005     2005           
=============================================
  Hits               1990     1990           
  Misses               15       15
Impacted Files Coverage Δ
src/urllib3/_async/connection.py 98.44% <ø> (ø) ⬆️
src/urllib3/util/connection.py 100% <ø> (ø) ⬆️

Copy link
Member

@RatanShreshtha RatanShreshtha left a comment

Choose a reason for hiding this comment

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

Looks good to me

@RatanShreshtha
Copy link
Member

I will raise the PR for upstream

@RatanShreshtha RatanShreshtha merged commit b5dc270 into python-trio:bleach-spike Nov 14, 2019
@pquentin pquentin deleted the merge-from-master-2019-10-30 branch November 14, 2019 10:39
@pquentin
Copy link
Member Author

Thank you for the review!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants