Fix compatibility regressions with Review Board 5 and Python 3.

Review Request #12440 — Created July 6, 2022 and submitted

Information

rbintegrations
release-3.x

Reviewers

rbintegrations was no longer working with Review Board 5, for a couple
reasons.

First, it used deprecated imports that Review Board 5 no longer
provides (as of this change, though that removal is being reverted
for now). These were attempting to import
reviewboard.integrations.Integration. We now use the
reviewboard.integrations.base.Integration import instead.

Second, we had calls in CircleCI to Django's deprecated
urlquote_plus() (which now just wraps quote_plus()) that were
failing due to passing in a Unicode string when a byte string was
expected. We now pass in a byte string.

In the process of fixing the urlquote_plus() issue, I found we were
convert a None value into the string 'None'. This seems to have been
a mistake, except the unit tests were matching that. I suspect this was
more a result of copy/paste than deliberate design.

This has been updated to just convert to an empty string and check
against that, which isn't really a whole lot better, but it's at least
more likely to trigger some meaningful error on CircleCI. A task has
been added to track this.

All unit tests pass.

Summary ID
Fix compatibility regressions with Review Board 5 and Python 3.
rbintegrations was no longer working with Review Board 5, for a couple reasons. First, it used deprecated imports that Review Board 5 no longer provides. These were attempting to import `reviewboard.integrations.Integration`. We now use the `reviewboard.integrations.base.Integration` import instead. Second, we had calls in CircleCI to Django's deprecated `urlquote_plus()` (which now just wraps `quote_plus()`) that were failing due to passing in a Unicode string when a byte string was expected. We now pass in a byte string. In the process of fixing the `urlquote_plus()` issue, I found we were convert a `None` value into the string `'None'`. This seems to have been a mistake, except the unit tests were matching that. I suspect this was more a result of copy/paste than deliberate design. This has been updated to just convert to an empty string and check against that, which isn't really a whole lot better, but it's at least more likely to trigger some meaningful error on CircleCI. A task has been added to track this.
3b2122c4992b758d5f0a5e8cf2384685ace682cc
david
  1. Ship It!
  2. 
      
maubin
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.x (3ac60d2)
Loading...