Opened 11 months ago

Closed 3 months ago

#30946 closed defect (fixed)

Port BridgeDB to Python 3

Reported by: phw Owned by: phw
Priority: Medium Milestone:
Component: Circumvention/BridgeDB Version:
Severity: Normal Keywords: python
Cc: sysrqb, cohosh, gaba Actual Points: 5.3
Parent ID: Points: 10
Reviewer: Sponsor:

Description

BridgeDB is written in Python 2.7, which will no longer be maintained past 2020. We should port BridgeDB to Python 3. This may involve quite some work given BridgeDB's large code base and the libraries it depends on.

Child Tickets

Change History (10)

comment:1 Changed 6 months ago by phw

Here's a summary of what I've accomplished so far:

  • I ran the tool futurize over BridgeDB's code, which can do simple code transformations from Python 2 to Python 3.
  • The switch to Python 3 resulted in several API incompatibilities in our dependencies. These are annoying to deal with but not a big issue.
  • The biggest issue is that strings and bytes can no longer be mixed in Python 3. We have hundreds of failed unit tests because of this, and fixing these isn't always as simple as turning a string into a bytes type. Sometimes, it requires thinking if a given function should return a string or a bytes type, and making sure that these choices are consistent throughout the code.

So far, I've spent 12 hours on this ticket and haven't made much progress, but progress shouldn't be linear: once I've addressed issues in BridgeDB's "critical path", significantly more unit tests should pass. I managed to get the unit tests from:

FAILED (skips=7, failures=40, errors=344, successes=666)

to

FAILED (skips=8, failures=31, errors=278, successes=737)

I expect this to take several more days of work.

comment:2 Changed 5 months ago by atagar

Hi phw. As discussed on #32900 I can lend a hand on this if you'd like. Cloning bridgedb commit 5bded87 when I run 'make coverage' I get...

FAILED (failures=5, errors=138, successes=164)

You mention a patch that gets us closer - where is it?

comment:3 in reply to:  2 Changed 5 months ago by phw

Replying to atagar:

Hi phw. As discussed on #32900 I can lend a hand on this if you'd like. Cloning bridgedb commit 5bded87 when I run 'make coverage' I get...

FAILED (failures=5, errors=138, successes=164)

You mention a patch that gets us closer - where is it?


My work-in-progress branch is available here:
https://github.com/NullHypothesis/bridgedb/tree/fix/30946

A word of warning: it's a mess, with (mostly) meaningless commit messages. I'm happy to squash it, and make it more meaningful if it helps.

comment:4 Changed 4 months ago by atagar

Hi Philipp. It took alot of work but I've finished porting BridgeDB to python 3. The port is available within my python3 branch...

https://github.com/atagar/bridgedb/commits/python3

Tests now pass with python 3.5 but has not been exercised in a runtime context. Its tests provide good coverage so should be close.

If tor would ever care to hire a BridgeDB maintainer to replace Isis let me know. The codebase relies excessively on catch-alls to mask underlying instability so there's quite a bit of room for stabilization.

comment:5 in reply to:  4 Changed 4 months ago by phw

Replying to atagar:

Hi Philipp. It took alot of work but I've finished porting BridgeDB to python 3. The port is available within my python3 branch...

https://github.com/atagar/bridgedb/commits/python3

Tests now pass with python 3.5 but has not been exercised in a runtime context. Its tests provide good coverage so should be close.


Thanks a lot, Damian! I am now building on your great patch set, to make it work in a runtime context.

If tor would ever care to hire a BridgeDB maintainer to replace Isis let me know. The codebase relies excessively on catch-alls to mask underlying instability so there's quite a bit of room for stabilization.


Our new anti-censorship team is responsible for BridgeDB and I'm its new maintainer. We will hopefully be able to do some general maintenance as part of our Sponsor 30 grant. Let me know if you have any specific suggestions on what kind of BridgeDB improvements you would like to see.

comment:6 Changed 4 months ago by phw

Status: assignedneeds_review

Hi Damian, can I ask you to take a look at my patch set, which fixes the remaining Python 3 issues? It's based on your great work and is available here:
https://github.com/NullHypothesis/bridgedb/commits/python3

Of particular importance is commit fcdc813 because it makes several changes to BridgeDB's HTTPS server code. To be honest, I'm having a hard time wrapping my head around the way strings and bytes are now used in BridgeDB and I was wondering if you can think of ways to simplify my patches.

comment:7 Changed 4 months ago by atagar

Hi Philipp. I skimmed over your branch (commit range fbc93cd..1c0281d) and it looks good to me. As you mentioned the vast majority is bytes/unicode normalization, and those look fine to me.

Minor thing but the shebangs are inconsistent...

% git diff fbc93cd..1c0281d | grep '+#!'
+#!/usr/bin/env python3.7
+#!/usr/bin/env python3

comment:8 in reply to:  7 Changed 4 months ago by phw

Owner: set to phw
Status: needs_reviewassigned

Replying to atagar:

Hi Philipp. I skimmed over your branch (commit range fbc93cd..1c0281d) and it looks good to me. As you mentioned the vast majority is bytes/unicode normalization, and those look fine to me.


Great, thanks for taking a look!

Minor thing but the shebangs are inconsistent...

% git diff fbc93cd..1c0281d | grep '+#!'
+#!/usr/bin/env python3.7
+#!/usr/bin/env python3


Oops, that's a good catch. I pushed a fix.

Regarding next steps: I will run BridgeDB locally, to see if I encounter any more runtime issues. Once this is looking good, I'll test-deploy it on polyanthum, to see what runtime issues remain.

comment:9 Changed 4 months ago by phw

Earlier today, I deployed my fix/30946 branch for BridgeDB and my fix/30946 branch for bridgedb-admin on polyanthum. I found a handful more string-related issues in moat's server code, which are now fixed. I was subsequently able to get bridges over HTTPS, moat, and email. Let's keep the branch running and see if any other issues emerge.

comment:10 Changed 3 months ago by phw

Actual Points: 5.3
Resolution: fixed
Status: assignedclosed

No more issues have surfaced after several days of running this branch, so it's time to wrap things up! I merged this patch set in commit c3a820d and f801cc1, released BridgeDB version 0.9.4, and deployed it on polyanthum.

Thanks again for your help, Damian!

Note: See TracTickets for help on using tickets.