launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22657
[Merge] lp:~cjwatson/launchpad/explicit-proxy-trac into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/explicit-proxy-trac into lp:launchpad with lp:~cjwatson/launchpad/explicit-proxy-bugzilla as a prerequisite.
Commit message:
Convert the Trac external bug tracker to urlfetch.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/explicit-proxy-trac/+merge/348433
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/explicit-proxy-trac into lp:launchpad.
=== modified file 'lib/lp/bugs/doc/externalbugtracker-trac-lp-plugin.txt'
--- lib/lp/bugs/doc/externalbugtracker-trac-lp-plugin.txt 2016-09-21 02:49:42 +0000
+++ lib/lp/bugs/doc/externalbugtracker-trac-lp-plugin.txt 2018-06-23 02:06:14 +0000
@@ -30,23 +30,22 @@
Trac to validate $token and return a Set-Cookie header.
>>> import random
+ >>> import re
+ >>> from six.moves.urllib_parse import (
+ ... urljoin,
+ ... urlsplit,
+ ... )
+ >>> from lp.bugs.tests.externalbugtracker import BugTrackerResponsesMixin
>>> from lp.services.verification.interfaces.logintoken import (
... ILoginTokenSet)
- >>> from lp.services.webapp.url import urlappend
>>> from lp.testing.dbuser import lp_dbuser
- >>> class FakeResponse:
- ... def __init__(self):
- ... self.headers = {}
-
- >>> class TestTracLPPlugin(TracLPPlugin):
- ... def urlopen(self, url, data=None):
+ >>> class TestTracLPPlugin(BugTrackerResponsesMixin, TracLPPlugin):
+ ... def _getCallback(self, request):
... with lp_dbuser():
- ... url = url.get_full_url()
- ... base_auth_url = urlappend(self.baseurl, 'launchpad-auth')
- ... if not url.startswith(base_auth_url + '/'):
- ... raise AssertionError("Unexpected URL: %s" % url)
- ... token_text = url.split('/')[-1]
+ ... login(ANONYMOUS)
+ ... url = urlsplit(request.url)
+ ... token_text = url.path.split('/')[-1]
... token = getUtility(ILoginTokenSet)[token_text]
... if token.tokentype.name != 'BUGTRACKER':
... raise AssertionError(
@@ -59,20 +58,24 @@
... cookie_string = (
... 'trac_auth=random_token-' + str(random.random()))
... self._xmlrpc_transport.setCookie(cookie_string)
- ... response = FakeResponse()
- ... response.headers['Set-Cookie'] = cookie_string
+ ... return 200, {'Set-Cookie': cookie_string}, ''
...
- ... return response
+ ... def addResponses(self, requests_mock):
+ ... requests_mock.add_callback(
+ ... 'GET',
+ ... re.compile(
+ ... re.escape(urljoin(self.baseurl, 'launchpad-auth/'))),
+ ... self._getCallback)
To generate the token, the internal XML-RPC server is used. By using the
-XML-RPC server rather than talking to the database directy means that we
-don't have to bother about commiting the transaction to make the token
+XML-RPC server rather than talking to the database directly means that we
+don't have to bother about committing the transaction to make the token
visible to Trac.
- >>> from cookielib import CookieJar
+ >>> from requests.cookies import RequestsCookieJar
>>> from lp.bugs.tests.externalbugtracker import (
... TestInternalXMLRPCTransport)
- >>> cookie_jar = CookieJar()
+ >>> cookie_jar = RequestsCookieJar()
>>> test_transport = TestTracXMLRPCTransport(
... 'http://example.com/', cookie_jar)
>>> trac = TestTracLPPlugin(
@@ -82,70 +85,57 @@
The method that authenticates with Trac is _authenticate().
- >>> trac._authenticate()
+ >>> with trac.responses():
+ ... trac._authenticate()
Using XML-RPC to generate token.
Successfully validated the token.
After it has been called, the XML-RPC transport will have its
auth_cookie attribute set.
- >>> test_transport.cookie_processor.cookiejar
- <...CookieJar[Cookie(version=0, name='trac_auth'...
-
-The XML-RPC transport's cookie_processor shares its cookiejar with the
-TracLPPlugin instance. This is so that the TracLPPlugin can use the
-cookiejar when authenticating with the remote Trac and then pass it to
-the XML-RPC transport for further use, meaning that there's no need to
-manually manipulate cookies.
-
- >>> test_transport.cookie_processor.cookiejar == trac._cookie_jar
+ >>> test_transport.cookie_jar
+ <RequestsCookieJar[Cookie(version=0, name='trac_auth'...
+
+The XML-RPC transport shares its cookiejar with the TracLPPlugin instance.
+This is so that the TracLPPlugin can use the cookiejar when authenticating
+with the remote Trac and then pass it to the XML-RPC transport for further
+use, meaning that there's no need to manually manipulate cookies.
+
+ >>> test_transport.cookie_jar == trac._cookie_jar
True
So if we look in the TracLPPlugin's CookieJar we'll see the same cookie:
>>> trac._cookie_jar
- <...CookieJar[Cookie(version=0, name='trac_auth'...
+ <RequestsCookieJar[Cookie(version=0, name='trac_auth'...
And altering the cookie in the TracLPPlugin's CookieJar will mean, of
course, that it's altered in the XML-RPC transport's CookieJar, too.
- >>> from cookielib import Cookie
- >>> new_cookie = Cookie(
- ... name="trac_auth",
- ... value="Look ma, a new cookie!",
- ... version=0, port=None, port_specified=False,
- ... domain='http://example.com', domain_specified=True,
- ... domain_initial_dot=None, path='', path_specified=False,
- ... secure=False, expires=False, discard=None, comment=None,
- ... comment_url=None, rest=None)
-
>>> trac._cookie_jar.clear()
- >>> trac._cookie_jar.set_cookie(new_cookie)
+ >>> _ = trac._cookie_jar.set(
+ ... 'trac_auth', 'Look ma, a new cookie!',
+ ... domain='http://example.com', path='')
>>> trac._cookie_jar
<...CookieJar[Cookie(version=0, name='trac_auth',
value='Look ma, a new cookie!'...>
- >>> test_transport.cookie_processor.cookiejar
+ >>> test_transport.cookie_jar
<...CookieJar[Cookie(version=0, name='trac_auth',
value='Look ma, a new cookie!'...>
If authentication fails, a BugTrackerAuthenticationError will be raised.
- >>> from urllib2 import HTTPError
- >>> class TestFailingTracLPPlugin(TracLPPlugin):
- ... def urlopen(self, url, data=None):
- ... raise HTTPError(url, 401, "Denied!", {}, None)
-
- >>> test_trac = TestFailingTracLPPlugin(
- ... 'http://example.com', xmlrpc_transport=test_transport,
- ... internal_xmlrpc_transport=TestInternalXMLRPCTransport(),
- ... cookie_jar=cookie_jar)
- >>> test_trac._authenticate()
+ >>> with trac.responses() as requests_mock:
+ ... requests_mock.reset()
+ ... requests_mock.add(
+ ... 'GET', re.compile(r'.*/launchpad-auth/.*'), status=401)
+ ... trac._authenticate()
Traceback (most recent call last):
...
BugTrackerAuthenticationError: http://example.com:
- HTTP Error 401: Denied!
+ 401 Client Error: Unauthorized
Current time
@@ -169,7 +159,8 @@
>>> test_transport.seconds_since_epoch = 1207706521 + HOUR
>>> test_transport.local_timezone = 'CET'
>>> test_transport.utc_offset = HOUR
- >>> trac.getCurrentDBTime()
+ >>> with trac.responses():
+ ... trac.getCurrentDBTime()
Using XML-RPC to generate token.
Successfully validated the token.
datetime.datetime(2008, 4, 9, 2, 2, 1, tzinfo=<UTC>)
@@ -187,7 +178,8 @@
sent again.
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> trac.getCurrentDBTime()
+ >>> with trac.responses():
+ ... trac.getCurrentDBTime()
Using XML-RPC to generate token.
Successfully validated the token.
datetime.datetime(2008, 4, 9, 2, 2, 1, tzinfo=<UTC>)
@@ -220,8 +212,8 @@
>>> bug_ids_to_check = ['1', '2', '3']
>>> last_checked = datetime(2008, 1, 1, 0, 0, 0)
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> sorted(trac.getModifiedRemoteBugs(
- ... bug_ids_to_check, last_checked))
+ >>> with trac.responses():
+ ... sorted(trac.getModifiedRemoteBugs(bug_ids_to_check, last_checked))
Using XML-RPC to generate token.
Successfully validated the token.
['1', '3']
@@ -231,8 +223,8 @@
>>> last_checked = datetime(2008, 2, 1, 0, 0, 0)
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> trac.getModifiedRemoteBugs(
- ... bug_ids_to_check, last_checked)
+ >>> with trac.responses():
+ ... trac.getModifiedRemoteBugs(bug_ids_to_check, last_checked)
Using XML-RPC to generate token.
Successfully validated the token.
['1']
@@ -242,8 +234,8 @@
>>> last_checked = datetime(2008, 5, 1, 0, 0, 0)
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> trac.getModifiedRemoteBugs(
- ... bug_ids_to_check, last_checked)
+ >>> with trac.responses():
+ ... trac.getModifiedRemoteBugs(bug_ids_to_check, last_checked)
Using XML-RPC to generate token.
Successfully validated the token.
[]
@@ -255,8 +247,8 @@
>>> bug_ids_to_check = ['1', '2', '3', '99', '100']
>>> last_checked = datetime(2008, 1, 1, 0, 0, 0)
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> sorted(trac.getModifiedRemoteBugs(
- ... bug_ids_to_check, last_checked))
+ >>> with trac.responses():
+ ... sorted(trac.getModifiedRemoteBugs(bug_ids_to_check, last_checked))
Using XML-RPC to generate token.
Successfully validated the token.
['1', '100', '3', '99']
@@ -281,7 +273,8 @@
>>> bugs_to_update = trac.getModifiedRemoteBugs(
... bug_ids_to_check, last_checked)
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> trac.initializeRemoteBugDB(bugs_to_update)
+ >>> with trac.responses():
+ ... trac.initializeRemoteBugDB(bugs_to_update)
Using XML-RPC to generate token.
Successfully validated the token.
@@ -368,7 +361,8 @@
>>> test_transport.expireCookie(test_transport.auth_cookie)
>>> bugs_to_update = ['1', '2', '3']
- >>> trac.initializeRemoteBugDB(bugs_to_update)
+ >>> with trac.responses():
+ ... trac.initializeRemoteBugDB(bugs_to_update)
Using XML-RPC to generate token.
Successfully validated the token.
@@ -410,7 +404,8 @@
>>> transaction.commit()
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> trac.fetchComments(remote_bug, ['1-1'])
+ >>> with trac.responses():
+ ... trac.fetchComments(remote_bug, ['1-1'])
Using XML-RPC to generate token.
Successfully validated the token.
@@ -512,8 +507,9 @@
>>> message_rfc822msgid = message.rfc822msgid
>>> transaction.commit()
- >>> remote_comment_id = trac.addRemoteComment(
- ... '3', message_text_contents, message_rfc822msgid)
+ >>> with trac.responses():
+ ... remote_comment_id = trac.addRemoteComment(
+ ... '3', message_text_contents, message_rfc822msgid)
Using XML-RPC to generate token.
Successfully validated the token.
@@ -553,7 +549,8 @@
getLaunchpadBugId() requires authentication.
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> launchpad_bug_id = trac.getLaunchpadBugId('3')
+ >>> with trac.responses():
+ ... launchpad_bug_id = trac.getLaunchpadBugId('3')
Using XML-RPC to generate token.
Successfully validated the token.
@@ -564,7 +561,9 @@
bug. setLaunchpadBugId() also requires authentication.
>>> test_transport.expireCookie(test_transport.auth_cookie)
- >>> trac.setLaunchpadBugId('3', 15, 'http://bugs.launchpad.dev/bugs/xxx')
+ >>> with trac.responses():
+ ... trac.setLaunchpadBugId(
+ ... '3', 15, 'http://bugs.launchpad.dev/bugs/xxx')
Using XML-RPC to generate token.
Successfully validated the token.
=== modified file 'lib/lp/bugs/doc/externalbugtracker-trac.txt'
--- lib/lp/bugs/doc/externalbugtracker-trac.txt 2016-07-04 17:08:29 +0000
+++ lib/lp/bugs/doc/externalbugtracker-trac.txt 2018-06-23 02:06:14 +0000
@@ -31,17 +31,14 @@
If the LP plugin is installed, the URL will return 401, since it fails
to validate the token.
- >>> import urllib2
- >>> class TracHavingLPPlugin401(Trac):
- ... def urlopen(self, url, data=None):
- ... print url
- ... raise urllib2.HTTPError(
- ... url, 401, "Unauthorized", None, None)
- >>> trac = TracHavingLPPlugin401('http://example.com/')
- >>> chosen_trac = trac.getExternalBugTrackerToUse()
- http://example.com/launchpad-auth/check
-
+ >>> import responses
>>> from lp.bugs.externalbugtracker.trac import TracLPPlugin
+
+ >>> trac = Trac('http://example.com/')
+ >>> with responses.RequestsMock() as requests_mock:
+ ... requests_mock.add(
+ ... 'GET', 'http://example.com/launchpad-auth/check', status=401)
+ ... chosen_trac = trac.getExternalBugTrackerToUse()
>>> isinstance(chosen_trac, TracLPPlugin)
True
>>> chosen_trac.baseurl
@@ -54,30 +51,13 @@
a valid token, which is very unlikely), is that the broken Trac will
not include a "trac_auth" cookie.
- >>> from StringIO import StringIO
- >>> from httplib import HTTPMessage
- >>> from urllib import addinfourl
-
- >>> class TracReturning200ForPageNotFound(Trac):
- ... def urlopen(self, url, data=None):
- ... print url
- ... return addinfourl(
- ... StringIO(''), HTTPMessage(StringIO('')), url)
-
- >>> class TracHavingLPPlugin200(Trac):
- ... def urlopen(self, url, data=None):
- ... print url
- ... return addinfourl(
- ... StringIO(''), HTTPMessage(
- ... StringIO('Set-Cookie: trac_auth=1234')), url)
-
The plain, non-plugin, external bug tracker is selected for broken
Trac installations:
- >>> trac = TracReturning200ForPageNotFound('http://example.com/')
- >>> chosen_trac = trac.getExternalBugTrackerToUse()
- http://example.com/launchpad-auth/check
-
+ >>> with responses.RequestsMock() as requests_mock:
+ ... requests_mock.add(
+ ... 'GET', 'http://example.com/launchpad-auth/check')
+ ... chosen_trac = trac.getExternalBugTrackerToUse()
>>> isinstance(chosen_trac, TracLPPlugin)
False
>>> chosen_trac.baseurl
@@ -86,10 +66,11 @@
In the event that our deliberately bogus token is considered valid,
the external bug tracker that groks the plugin is selected:
- >>> trac = TracHavingLPPlugin200('http://example.com/')
- >>> chosen_trac = trac.getExternalBugTrackerToUse()
- http://example.com/launchpad-auth/check
-
+ >>> with responses.RequestsMock() as requests_mock:
+ ... requests_mock.add(
+ ... 'GET', 'http://example.com/launchpad-auth/check',
+ ... headers={'Set-Cookie': 'trac_auth=1234'})
+ ... chosen_trac = trac.getExternalBugTrackerToUse()
>>> isinstance(chosen_trac, TracLPPlugin)
True
>>> chosen_trac.baseurl
@@ -97,15 +78,10 @@
If a 404 is returned, the normal Trac instance is returned.
- >>> class TracNotHavingLPPlugin(Trac):
- ... def urlopen(self, url, data=None):
- ... print url
- ... raise urllib2.HTTPError(
- ... url, 404, "Not found", None, None)
- >>> trac = TracNotHavingLPPlugin('http://example.com/')
- >>> chosen_trac = trac.getExternalBugTrackerToUse()
- http://example.com/launchpad-auth/check
-
+ >>> with responses.RequestsMock() as requests_mock:
+ ... requests_mock.add(
+ ... 'GET', 'http://example.com/launchpad-auth/check', status=404)
+ ... chosen_trac = trac.getExternalBugTrackerToUse()
>>> chosen_trac is trac
True
@@ -113,14 +89,12 @@
instance. It will deal with the connection error later, if the situation
persists.
- >>> class TracWithConnectionError(Trac):
- ... def urlopen(self, url, data=None):
- ... print url
- ... raise urllib2.URLError("Connection timed out")
- >>> trac = TracWithConnectionError('http://example.com/')
- >>> chosen_trac = trac.getExternalBugTrackerToUse()
- http://example.com/launchpad-auth/check
-
+ >>> from requests.exceptions import ConnectTimeout
+ >>> with responses.RequestsMock() as requests_mock:
+ ... requests_mock.add(
+ ... 'GET', 'http://example.com/launchpad-auth/check',
+ ... body=ConnectTimeout())
+ ... chosen_trac = trac.getExternalBugTrackerToUse()
>>> chosen_trac is trac
True
@@ -172,18 +146,19 @@
local variable for later use.
We use a test-oriented implementation for the purposes of these tests, which
-overrides ExternalBugTracker.urlopen() so that we don't have to rely on a
-working network connection.
+avoids relying on a network connection.
>>> from lp.bugs.tests.externalbugtracker import TestTrac
>>> trac = TestTrac(u'http://test.trac/')
- >>> trac.initializeRemoteBugDB([1])
+ >>> with trac.responses():
+ ... trac.initializeRemoteBugDB([1])
>>> sorted(trac.bugs.keys())
[1]
If we initialize with a different set of keys we overwrite the first set:
- >>> trac.initializeRemoteBugDB([6,7,8,9,10,11,12])
+ >>> with trac.responses():
+ ... trac.initializeRemoteBugDB([6, 7, 8, 9, 10, 11, 12])
>>> sorted(trac.bugs.keys())
[6, 7, 8, 9, 10, 11, 12]
@@ -198,36 +173,41 @@
>>> trac.batch_query_threshold
10
- >>> trac.trace_calls = True
- >>> trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
- CALLED urlopen(u'http://test.trac/ticket/6?format=csv')
- CALLED urlopen(u'http://test.trac/ticket/7?format=csv')
- CALLED urlopen(u'http://test.trac/ticket/8?format=csv')
- CALLED urlopen(u'http://test.trac/ticket/9?format=csv')
- CALLED urlopen(u'http://test.trac/ticket/10?format=csv')
+ >>> with trac.responses(trace_calls=True):
+ ... trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
+ GET http://test.trac/ticket/6
+ GET http://test.trac/ticket/6?format=csv
+ GET http://test.trac/ticket/6?format=csv
+ GET http://test.trac/ticket/7?format=csv
+ GET http://test.trac/ticket/8?format=csv
+ GET http://test.trac/ticket/9?format=csv
+ GET http://test.trac/ticket/10?format=csv
If there are more than batch_query_threshold bugs to update then they are
fetched as a batch:
>>> trac.batch_query_threshold = 4
- >>> trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
- CALLED urlopen(u'http://test.trac/query?id=6&id=7...&format=csv')
+ >>> with trac.responses(trace_calls=True):
+ ... trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
+ GET http://test.trac/query?id=6&id=7...&format=csv
The batch updating method will also be used in cases where the Trac instance
doesn't support CSV exports of individual tickets:
>>> trac.batch_query_threshold = 10
- >>> trac.supports_single_exports = False
- >>> trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
- CALLED urlopen(u'http://test.trac/query?id=6&id=7...&format=csv')
+ >>> with trac.responses(trace_calls=True, supports_single_exports=False):
+ ... trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
+ GET http://test.trac/ticket/6
+ GET http://test.trac/ticket/6?format=csv
+ GET http://test.trac/query?id=6&id=7...&format=csv
If, when using the batch export method, the Trac instance comes across
invalid data, it will raise an UnparsableBugData exception. We will
force our trac instance to use invalid data for the purposes of this
test.
- >>> trac.csv_export_file = 'trac_example_broken_ticket_export.csv'
- >>> trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
+ >>> with trac.responses(broken=True):
+ ... trac.initializeRemoteBugDB([6, 7, 8, 9, 10])
Traceback (most recent call last):
...
UnparsableBugData: External bugtracker http://test.trac does not
@@ -236,8 +216,8 @@
This is also true of the single bug export mode.
- >>> trac.supports_single_exports = True
- >>> trac.initializeRemoteBugDB([6])
+ >>> with trac.responses(broken=True):
+ ... trac.initializeRemoteBugDB([6])
Traceback (most recent call last):
...
UnparsableBugData: External bugtracker http://test.trac does not
@@ -256,15 +236,13 @@
method to retrieve the CSV data from the remote Trac instance. This
method accepts a URL from which to retrieve the data as a parameter.
- >>> trac.supports_single_exports = False
- >>> trac.csv_export_file = None
-
- >>> query_url = 'http://example.com/query?id=%s&format=csv'
+ >>> query_url = 'http://test.trac/query?id=%s&format=csv'
>>> query_string = '&id='.join(['1', '2', '3', '4', '5'])
>>> query_url = query_url % query_string
- >>> remote_bugs = trac._fetchBugData(query_url)
- CALLED urlopen('http://example.com/query?id=1&id=2...&format=csv')
+ >>> with trac.responses(trace_calls=True, supports_single_exports=False):
+ ... remote_bugs = trac._fetchBugData(query_url)
+ GET http://test.trac/query?id=1&id=2...&format=csv
However, _fetchBugData() doesn't actually check the results it returns
except for checking that they are valid Trac CSV exports. in this case,
@@ -277,8 +255,8 @@
If _fetchBugData() receives a response that isn't a valid Trac CSV
export, it will raise an UnparsableBugData error.
- >>> trac.csv_export_file = 'trac_example_broken_ticket_export.csv'
- >>> trac._fetchBugData(query_url)
+ >>> with trac.responses(broken=True):
+ ... trac._fetchBugData(query_url)
Traceback (most recent call last):
...
UnparsableBugData: External bugtracker http://test.trac does not
@@ -322,7 +300,9 @@
>>> bug_watch_updater = CheckwatchesMaster(
... txn, FakeLogger())
>>> trac = TestTrac(example_bug_tracker.baseurl)
- >>> bug_watch_updater.updateBugWatches(trac, example_bug_tracker.watches)
+ >>> with trac.responses():
+ ... bug_watch_updater.updateBugWatches(
+ ... trac, example_bug_tracker.watches)
INFO Updating 1 watches for 1 bugs on http://bugs.some.where
>>> for bug_watch in example_bug_tracker.watches:
@@ -364,10 +344,11 @@
... remotebug=str(remote_bug_id))
... bug_watches[remote_bug_id] = bug_watch.id
- >>> trac.trace_calls = True
- >>> bug_watch_updater.updateBugWatches(trac, example_bug_tracker.watches)
+ >>> with trac.responses(trace_calls=True):
+ ... bug_watch_updater.updateBugWatches(
+ ... trac, example_bug_tracker.watches)
INFO Updating 12 watches for 12 bugs on http://bugs.some.where
- CALLED urlopen(u'http://.../query?id=...
+ GET http://bugs.some.where/query?id=...
>>> for remote_bug_id in sorted(bug_watches.keys()):
... bug_watch = getUtility(IBugWatchSet).get(
@@ -415,8 +396,8 @@
u'new'
>>> trac.batch_query_threshold = 0
- >>> trac.trace_calls = False
- >>> bug_watch_updater.updateBugWatches(trac, [bug_watch])
+ >>> with trac.responses():
+ ... bug_watch_updater.updateBugWatches(trac, [bug_watch])
INFO Updating 1 watches for 1 bugs on http://bugs.some.where
>>> bug_watch.lastchanged == old_last_changed
=== modified file 'lib/lp/bugs/externalbugtracker/trac.py'
--- lib/lp/bugs/externalbugtracker/trac.py 2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/externalbugtracker/trac.py 2018-06-23 02:06:14 +0000
@@ -6,16 +6,16 @@
__metaclass__ = type
__all__ = ['Trac', 'TracLPPlugin']
-from Cookie import SimpleCookie
-from cookielib import CookieJar
import csv
from datetime import datetime
from email.utils import parseaddr
import time
-import urllib2
import xmlrpclib
+from mimeparse import parse_mime_type
import pytz
+import requests
+from requests.cookies import RequestsCookieJar
from zope.component import getUtility
from zope.interface import implementer
@@ -24,13 +24,13 @@
BugNotFound,
BugTrackerAuthenticationError,
BugTrackerConnectError,
- ExternalBugTracker,
+ ExternalBugTrackerRequests,
InvalidBugId,
LookupTree,
UnknownRemoteStatusError,
UnparsableBugData,
)
-from lp.bugs.externalbugtracker.xmlrpc import UrlLib2Transport
+from lp.bugs.externalbugtracker.xmlrpc import RequestsTransport
from lp.bugs.interfaces.bugtask import (
BugTaskImportance,
BugTaskStatus,
@@ -44,6 +44,10 @@
from lp.services.config import config
from lp.services.database.isolation import ensure_no_transaction
from lp.services.messages.interfaces.message import IMessageSet
+from lp.services.timeout import (
+ override_timeout,
+ urlfetch,
+ )
from lp.services.webapp.url import urlappend
# Symbolic constants used for the Trac LP plugin.
@@ -56,7 +60,7 @@
FAULT_TICKET_NOT_FOUND = 1001
-class Trac(ExternalBugTracker):
+class Trac(ExternalBugTrackerRequests):
"""An ExternalBugTracker instance for handling Trac bugtrackers."""
ticket_url = 'ticket/%i?format=csv'
@@ -69,15 +73,16 @@
# Any token will do.
auth_url = urlappend(base_auth_url, 'check')
try:
- response = self.urlopen(auth_url)
- except urllib2.HTTPError as error:
+ with override_timeout(config.checkwatches.default_socket_timeout):
+ response = urlfetch(auth_url, trust_env=False, use_proxy=True)
+ except requests.HTTPError as e:
# If the error is HTTP 401 Unauthorized then we're
# probably talking to the LP plugin.
- if error.code == 401:
+ if e.response.status_code == 401:
return TracLPPlugin(self.baseurl)
else:
return self
- except urllib2.URLError as error:
+ except requests.RequestException:
return self
else:
# If the response contains a trac_auth cookie then we're
@@ -85,9 +90,8 @@
# the remote system will authorize the bogus auth token we
# sent, so this check is really intended to detect broken
# Trac instances that return HTTP 200 for a missing page.
- for set_cookie in response.headers.getheaders('Set-Cookie'):
- cookie = SimpleCookie(set_cookie)
- if 'trac_auth' in cookie:
+ for cookie in response.cookies:
+ if cookie.name == 'trac_auth':
return TracLPPlugin(self.baseurl)
else:
return self
@@ -98,31 +102,37 @@
:bug_ids: A list of bug IDs that we can use for discovery purposes.
"""
- valid_ticket = False
html_ticket_url = '%s/%s' % (
self.baseurl, self.ticket_url.replace('?format=csv', ''))
- bug_ids = list(bug_ids)
- while not valid_ticket and len(bug_ids) > 0:
+ for bug_id in bug_ids:
try:
- # We try to retrive the ticket in HTML form, since that will
- # tell us whether or not it is actually a valid ticket
- ticket_id = int(bug_ids.pop())
- self._fetchPage(html_ticket_url % ticket_id)
- except (ValueError, urllib2.HTTPError):
- # If we get an HTTP error we can consider the ticket to be
- # invalid. If we get a ValueError then the ticket_id couldn't
- # be identified and it's of no use to us anyway.
+ # We try to retrieve the ticket in HTML form, since that
+ # will tell us whether or not it is actually a valid ticket.
+ ticket_id = int(bug_id)
+ self._getPage(html_ticket_url % ticket_id)
+ except BugTrackerConnectError as e:
+ if isinstance(e.error, requests.HTTPError):
+ # We can consider the ticket to be invalid.
+ pass
+ else:
+ raise
+ except ValueError:
+ # The ticket_id couldn't be identified and it's of no use to
+ # us anyway.
pass
else:
# If we didn't get an error we can try to get the ticket in
# CSV form. If this fails then we can consider single ticket
# exports to be unsupported.
try:
- csv_data = self._fetchPage(
- "%s/%s" % (self.baseurl, self.ticket_url % ticket_id))
- return csv_data.headers.subtype == 'csv'
- except (urllib2.HTTPError, urllib2.URLError):
+ response = self._getPage(
+ "%s/%s" %
+ (self.baseurl, self.ticket_url % ticket_id))
+ subtype = parse_mime_type(
+ response.headers.get('Content-Type', ''))[1]
+ return subtype == 'csv'
+ except BugTrackerConnectError:
return False
else:
# If we reach this point then we likely haven't had any valid
@@ -140,7 +150,7 @@
"""
# We read the remote bugs into a list so that we can check that
# the data we're getting back from the remote server are valid.
- csv_reader = csv.DictReader(self._fetchPage(query_url))
+ csv_reader = csv.DictReader(self._getPage(query_url).iter_lines())
remote_bugs = [csv_reader.next()]
# We consider the data we're getting from the remote server to
@@ -320,9 +330,9 @@
super(TracLPPlugin, self).__init__(baseurl)
if cookie_jar is None:
- cookie_jar = CookieJar()
+ cookie_jar = RequestsCookieJar()
if xmlrpc_transport is None:
- xmlrpc_transport = UrlLib2Transport(baseurl, cookie_jar)
+ xmlrpc_transport = RequestsTransport(baseurl, cookie_jar)
self._cookie_jar = cookie_jar
self._xmlrpc_transport = xmlrpc_transport
@@ -332,8 +342,10 @@
self._server = xmlrpclib.ServerProxy(
xmlrpc_endpoint, transport=self._xmlrpc_transport)
- self.url_opener = urllib2.build_opener(
- urllib2.HTTPCookieProcessor(cookie_jar))
+ def makeRequest(self, method, url, **kwargs):
+ """See `ExternalBugTracker`."""
+ return super(TracLPPlugin, self).makeRequest(
+ method, url, cookies=self._cookie_jar, **kwargs)
@ensure_no_transaction
@needs_authentication
@@ -364,7 +376,7 @@
auth_url = urlappend(base_auth_url, token_text)
try:
- self._fetchPage(auth_url)
+ self._getPage(auth_url)
except BugTrackerConnectError as e:
raise BugTrackerAuthenticationError(self.baseurl, e.error)
=== modified file 'lib/lp/bugs/tests/externalbugtracker.py'
--- lib/lp/bugs/tests/externalbugtracker.py 2018-06-23 02:06:14 +0000
+++ lib/lp/bugs/tests/externalbugtracker.py 2018-06-23 02:06:14 +0000
@@ -52,10 +52,7 @@
LP_PLUGIN_METADATA_AND_COMMENTS,
LP_PLUGIN_METADATA_ONLY,
)
-from lp.bugs.externalbugtracker.xmlrpc import (
- RequestsTransport,
- UrlLib2Transport,
- )
+from lp.bugs.externalbugtracker.xmlrpc import RequestsTransport
from lp.bugs.interfaces.bugtask import (
BugTaskImportance,
BugTaskStatus,
@@ -1199,44 +1196,45 @@
requests_mock.add('POST', re.compile(re.escape(self.baseurl)))
-class TestTrac(Trac):
- """Trac ExternalBugTracker for testing purposes.
-
- It overrides urlopen, so that access to a real Trac instance isn't needed,
- and supportsSingleExports so that the tests don't fail due to the lack of
- a network connection. Also, it overrides the default batch_query_threshold
- for the sake of making test data sane.
- """
+class TestTrac(BugTrackerResponsesMixin, Trac):
+ """Trac ExternalBugTracker for testing purposes."""
# We remove the batch_size limit for the purposes of the tests so
# that we can test batching and not batching correctly.
batch_size = None
batch_query_threshold = 10
csv_export_file = None
- supports_single_exports = True
- trace_calls = False
def getExternalBugTrackerToUse(self):
return self
- def supportsSingleExports(self, bug_ids):
- """See `Trac`."""
- return self.supports_single_exports
-
- def urlopen(self, url, data=None):
- file_path = os.path.join(os.path.dirname(__file__), 'testfiles')
- url = url.get_full_url()
- if self.trace_calls:
- print "CALLED urlopen(%r)" % (url)
-
- if self.csv_export_file is not None:
- csv_export_file = self.csv_export_file
- elif re.match('.*/ticket/[0-9]+\?format=csv$', url):
- csv_export_file = 'trac_example_single_ticket_export.csv'
- else:
- csv_export_file = 'trac_example_ticket_export.csv'
-
- return open(file_path + '/' + csv_export_file, 'r')
+ def _getCallback(self, request, supports_single_exports):
+ url = urlsplit(request.url)
+ if parse_qs(url.query).get('format') == ['csv']:
+ mimetype = 'text/csv'
+ if url.path.startswith('/ticket/'):
+ if not supports_single_exports:
+ return 404, {}, ''
+ body = read_test_file('trac_example_single_ticket_export.csv')
+ else:
+ body = read_test_file('trac_example_ticket_export.csv')
+ else:
+ mimetype = 'text/html'
+ body = ''
+ return 200, {'Content-Type': mimetype}, body
+
+ def addResponses(self, requests_mock, supports_single_exports=True,
+ broken=False):
+ url_pattern = re.compile(re.escape(self.baseurl))
+ if broken:
+ requests_mock.add(
+ 'GET', url_pattern,
+ body=read_test_file('trac_example_broken_ticket_export.csv'))
+ else:
+ requests_mock.add_callback(
+ 'GET', url_pattern,
+ lambda request: self._getCallback(
+ request, supports_single_exports))
class MockTracRemoteBug:
@@ -1297,7 +1295,7 @@
return comment
-class TestTracXMLRPCTransport(UrlLib2Transport):
+class TestTracXMLRPCTransport(RequestsTransport):
"""An XML-RPC transport to be used when testing Trac."""
remote_bugs = {}
@@ -1313,8 +1311,12 @@
@property
def auth_cookie(self):
- cookies = self.cookie_processor.cookiejar._cookies
- return cookies.get('example.com', {}).get('', {}).get('trac_auth')
+ for cookie in self.cookie_jar:
+ if (cookie.domain == 'example.com' and cookie.path == '' and
+ cookie.name == 'trac_auth'):
+ return cookie
+ else:
+ return None
@property
def has_valid_auth_cookie(self):
=== modified file 'setup.py'
--- setup.py 2018-06-06 09:01:15 +0000
+++ setup.py 2018-06-23 02:06:14 +0000
@@ -201,6 +201,7 @@
'python-debian',
'python-keystoneclient',
'python-memcached',
+ 'python-mimeparse',
'python-openid',
'python-subunit',
'python-swiftclient',
Follow ups
