Update treq

[wsanchez]

Update treq, which is necessary to address build failures against Twisted trunk.

[wsanchez]

The latest release of treq causes build failures due to use of twisted.python.compat._PY3, so this uses the current latest commit on master.

That still fails due to #339.

[wsanchez]

twisted/treq#293 requests a Treq release.

[adiroiban]

Does this draft require a review?

I am just trying to clean my https://github.com/pulls/review-requested queue :)

[twm]

@adiroiban I believe that it needs to be changed to reference treq 20.9.0 first.

[wsanchez]

@twm updated, but we still have #339.

Our options here are (a) fix treq to do what @glyph thinks is correct regarding + (which presumably means fixing python-hyper/hyperlink#129) or (b) changing test_form.py to stop caring what @glyph thinks, or (c) stop using treq in our test suite.

[twm]

@wsanchez I think that the ideal outcome is to fix Hyperlink. Its current behavior is to corrupt a common sequence in the most common type of URL. I definitely wouldn't have made treq pass all URLs through it if I'd known of that behavior, and it makes Hyperlink totally unsuited for use on the server side, as in Twisted #9359. It seems like a two birds with one stone situation to me.

As a fallback, I could make treq avoid using Hyperlink, which has also caused other problems (treq#303). That would fix it in this Klein case, but not help twisted/twisted#970.

[wsanchez]

@twm agreed. Like Klein, Hyperlink lacks active developers who will review code, so… if you submit a PR, I can review and merge, then cajole Mahmoud to make a release.

[glyph]

I'd really like to get it fixed & released in Hyperlink. I'm on vacation (let me pause just a moment to take a sip out of my mug here) so I might have a few spare cycles to help get this over the line in the next couple of weeks. I am working a couple of days, and I am planning to spend the rest of it playing World of Warcraft, but I will try to have a few minutes between raids.

[twm]

I gave fixing it in treq a shot. Unfortunately even the raw hyperlink.URL still mangles in this way. I'd have to back out treq's internal use of Hyperlink entirely to avoid this issue. This PR has a test to reproduce.

Meanwhile, both Hyperlink and treq are having CI issues. Hyperlink's issue is Travis (I guess builds pass... eventually?) and treq's is... several things, including Travis and OpenSSL. I filed some PRs:

• GitHub Actions CI config treq#308 (this is in code-coverage-tool hell)
• GitHub Actions CI python-hyper/hyperlink#142 (this isn't working at all, and I've no idea why; perhaps because I don't have write on the repo?)

There are several other treq PRs that need to go in before release, all small. I've started poking at a real fix in Hyperlink, but no promises there.

[glyph]

@twm Welp, right at the end of vacation, I did manage to squeeze this in: python-hyper/hyperlink#145

[twm]

@glyph I submitted python-hyper/hyperlink#146 to your PR, which also fixes the issue for hyperlink.URL.

[glyph]

@wsanchez @twm would one of you mind updating this PR to bump the hyperlink pin to point at trunk, just to get CI passing? We'll probably still want to put it on hold

[codecov]

Codecov Report

Merging #401 (c917555) into master (6744362) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #401   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files          45       45           
  Lines        3850     3850           
  Branches      249      249           
=======================================
  Hits         3794     3794           
  Misses         39       39           
  Partials       17       17           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6744362...c917555. Read the comment docs.

[twm]

It seems like the build timeouts may be too short?

[glyph]

It seems like the build timeouts may be too short?

It shouldn't take 30 minutes to run the unit tests. I think the problem is something broken with the usage of Hypothesis? I get errors locally about it generating too many tests. (I think that Klein is probably the wrong layer for most of these Hypothesis tests anyway; mostly they're testing HTTP parsing and URL stuff that should be handled by Hyperlink, Twisted, or Hyper H2, I think).

[wsanchez]

(I think that Klein is probably the wrong layer for most of these Hypothesis tests anyway; mostly they're testing HTTP parsing and URL stuff that should be handled by Hyperlink, Twisted, or Hyper H2, I think).

These are moving to Hyperlink, but we do use the strategies, so I need to dig into why sometimes they stall out, but that fix will end up being in Hyperlink.

[wsanchez]

OK I've updated Hyperlink and that seems to work without an update to Treq, which we can update out of band from this PR when it releases.

[adiroiban]

All good. Thanks.

A minor comment.

Reduce the name "Unit test with Python on Ubuntu with Twisted current" is it make is harder to see the duration from one go:

[glyph]

🎉