Merge lp:~mwhudson/launchpad/much-faster-rewrite-map into lp:launchpad/db-devel
- much-faster-rewrite-map
- Merge into db-devel
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Tim Penhey | ||||
| Approved revision: | no longer in the source branch. | ||||
| Merged at revision: | not available | ||||
| Proposed branch: | lp:~mwhudson/launchpad/much-faster-rewrite-map | ||||
| Merge into: | lp:launchpad/db-devel | ||||
| Diff against target: | None lines | ||||
| To merge this branch: | bzr merge lp:~mwhudson/launchpad/much-faster-rewrite-map | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Tim Penhey (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Description of the change
| Michael Hudson-Doyle (mwhudson) wrote : | # |
| Tim Penhey (thumper) wrote : | # |
review needs-info
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -113,8 +113,8 @@
> def __init__(self):
> self.buffer = StringIO()
>
> - def message(self, prefix, *stuff, **kw):
> - self.buffer.
> + def message(self, prefix, msg, *stuff, **kw):
> + self.buffer.
What is the impact of other code using message?
This to me looks like it'll brake things.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -5,38 +5,71 @@
> """
>
> import time
> -import xmlrpclib
>
> from bzrlib import urlutils
>
> -from lp.codehosting.vfs import (
> - branch_id_to_path, BranchFileSyste
> +from canonical.
> + IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
> +from zope.component import getUtility
> +from lp.code.
> +from lp.codehosting.vfs import branch_id_to_path
> +
> from canonical.config import config
> -from lp.code.
> - BRANCH_TRANSPORT, LAUNCHPAD_
> -from canonical.
> -from canonical.
>
> __all__ = ['BranchRewriter']
>
>
> class BranchRewriter:
>
> - def __init__(self, logger, proxy):
> + def __init__(self, logger, _now=None):
> """
>
> :param logger: Logger than messages about what the rewriter is
doing
> will be sent to.
> :param proxy: A blocking proxy for a branchfilesystem endpoint.
> """
> + if _now is None:
> + self._now = time.time
> + else:
> + self._now = _now
> self.logger = logger
> - self.client = BranchFileSyste
10.0)
> + self.store = getUtility(
SLAVE_FLAVOR)
> + self._cache = {}
>
> def _codebrowse_
> return urlutils.join(
> config.
> path)
>
> + def _getBranchIdAnd
> + """Return the branch id and trailing path for 'location'.
> +
> + In addition this method returns whether the answer can from the
cache
> + or from the database.
> + """
> + parts = location[
> + prefixes = []
> + for i in range(1, len(parts) + 1):
> + prefix = '/'.join(parts[:i])
> + if prefix in self._cache:
> + branch_id, inserted_time = self._cache[prefix]
> + if (self._now() < inserted_time +
> + config.
> + trailing = location[
> + return branch_id, trailing, "HIT"
> + prefixes.
> + ...
| Michael Hudson-Doyle (mwhudson) wrote : | # |
Tim Penhey wrote:
> Review: Needs Information
> review needs-info
>
>> === modified file 'lib/canonical/
>> --- lib/canonical/
>> +++ lib/canonical/
>> @@ -113,8 +113,8 @@
>> def __init__(self):
>> self.buffer = StringIO()
>>
>> - def message(self, prefix, *stuff, **kw):
>> - self.buffer.
>> + def message(self, prefix, msg, *stuff, **kw):
>> + self.buffer.
>
> What is the impact of other code using message?
It means that logger.info('%f', 0.1) doesn't break.
> This to me looks like it'll brake things.
I don't think it will. ec2test will confirm :)
>> === modified file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -5,38 +5,71 @@
>> """
>>
>> import time
>> -import xmlrpclib
>>
>> from bzrlib import urlutils
>>
>> -from lp.codehosting.vfs import (
>> - branch_id_to_path, BranchFileSyste
>> +from canonical.
>> + IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
>> +from zope.component import getUtility
>> +from lp.code.
>> +from lp.codehosting.vfs import branch_id_to_path
>> +
>> from canonical.config import config
>> -from lp.code.
>> - BRANCH_TRANSPORT, LAUNCHPAD_
>> -from canonical.
>> -from canonical.
>>
>> __all__ = ['BranchRewriter']
>>
>>
>> class BranchRewriter:
>>
>> - def __init__(self, logger, proxy):
>> + def __init__(self, logger, _now=None):
>> """
>>
>> :param logger: Logger than messages about what the rewriter is
> doing
>> will be sent to.
>> :param proxy: A blocking proxy for a branchfilesystem endpoint.
>> """
>> + if _now is None:
>> + self._now = time.time
>> + else:
>> + self._now = _now
>> self.logger = logger
>> - self.client = BranchFileSyste
> 10.0)
>> + self.store = getUtility(
> SLAVE_FLAVOR)
>> + self._cache = {}
>>
>> def _codebrowse_
>> return urlutils.join(
>> config.
>> path)
>>
>> + def _getBranchIdAnd
>> + """Return the branch id and trailing path for 'location'.
>> +
>> + In addition this method returns whether the answer can from the
> cache
>> + or from the database.
>> + """
>> + parts = location[
>> + prefixes = []
>> + for i in range(1, len(parts) + 1):
>> + prefix = '/'.join(parts[:i])
>> + if prefix in self._cache:
>> + branch_id, inserted_time = self._cache[prefix]
>> + if (self._now() < inserted_tim...
| 1 | === modified file 'lib/lp/codehosting/rewrite.py' | |||
| 2 | --- lib/lp/codehosting/rewrite.py 2009-07-22 03:24:19 +0000 | |||
| 3 | +++ lib/lp/codehosting/rewrite.py 2009-07-22 04:41:03 +0000 | |||
| 4 | @@ -59,16 +59,15 @@ | |||
| 5 | 59 | return branch_id, trailing, "HIT" | 59 | return branch_id, trailing, "HIT" |
| 6 | 60 | prefixes.append(prefix) | 60 | prefixes.append(prefix) |
| 7 | 61 | result = self.store.find( | 61 | result = self.store.find( |
| 14 | 62 | Branch, | 62 | (Branch.id, Branch.unique_name), |
| 15 | 63 | Branch.unique_name.is_in(prefixes), Branch.private == False) | 63 | Branch.unique_name.is_in(prefixes), Branch.private == False).one() |
| 16 | 64 | try: | 64 | if result is None: |
| 11 | 65 | branch_id, unique_name = result.values( | ||
| 12 | 66 | Branch.id, Branch.unique_name).next() | ||
| 13 | 67 | except StopIteration: | ||
| 17 | 68 | return None, None, "MISS" | 65 | return None, None, "MISS" |
| 21 | 69 | self._cache[unique_name] = (branch_id, self._now()) | 66 | else: |
| 22 | 70 | trailing = location[len(unique_name) + 1:] | 67 | branch_id, unique_name = result |
| 23 | 71 | return branch_id, trailing, "MISS" | 68 | self._cache[unique_name] = (branch_id, self._now()) |
| 24 | 69 | trailing = location[len(unique_name) + 1:] | ||
| 25 | 70 | return branch_id, trailing, "MISS" | ||
| 26 | 72 | 71 | ||
| 27 | 73 | def rewriteLine(self, resource_location): | 72 | def rewriteLine(self, resource_location): |
| 28 | 74 | """Rewrite 'resource_location' to a more concrete location. | 73 | """Rewrite 'resource_location' to a more concrete location. |
| 29 | 75 | 74 | ||
| 30 | === modified file 'lib/lp/codehosting/tests/test_rewrite.py' | |||
| 31 | --- lib/lp/codehosting/tests/test_rewrite.py 2009-07-22 03:23:28 +0000 | |||
| 32 | +++ lib/lp/codehosting/tests/test_rewrite.py 2009-07-22 04:56:35 +0000 | |||
| 33 | @@ -161,12 +161,10 @@ | |||
| 34 | 161 | self.factory.makeProductBranch(), | 161 | self.factory.makeProductBranch(), |
| 35 | 162 | self.factory.makePersonalBranch(), | 162 | self.factory.makePersonalBranch(), |
| 36 | 163 | self.factory.makePackageBranch()] | 163 | self.factory.makePackageBranch()] |
| 43 | 164 | input = [ | 164 | input_lines = [ |
| 44 | 165 | "/%s/.bzr/README" % branch.unique_name | 165 | "/%s/.bzr/README" % branch.unique_name for branch in branches] + [ |
| 45 | 166 | for branch in branches] + [ | 166 | "/%s/changes" % branch.unique_name for branch in branches] |
| 46 | 167 | "/%s/changes" % branch.unique_name | 167 | expected_lines = [ |
| 41 | 168 | for branch in branches] | ||
| 42 | 169 | expected = [ | ||
| 47 | 170 | 'file:///var/tmp/bzrsync/%s/.bzr/README' | 168 | 'file:///var/tmp/bzrsync/%s/.bzr/README' |
| 48 | 171 | % branch_id_to_path(branch.id) | 169 | % branch_id_to_path(branch.id) |
| 49 | 172 | for branch in branches] + [ | 170 | for branch in branches] + [ |
| 50 | @@ -178,15 +176,17 @@ | |||
| 51 | 178 | proc = subprocess.Popen( | 176 | proc = subprocess.Popen( |
| 52 | 179 | [script_file], stdin=subprocess.PIPE, stdout=subprocess.PIPE, | 177 | [script_file], stdin=subprocess.PIPE, stdout=subprocess.PIPE, |
| 53 | 180 | stderr=subprocess.PIPE, bufsize=0) | 178 | stderr=subprocess.PIPE, bufsize=0) |
| 58 | 181 | proc.stdin.write('\n'.join(input) + '\n') | 179 | output_lines = [] |
| 59 | 182 | output = [] | 180 | # For each complete line of input, the script should, without |
| 60 | 183 | for i in range(len(input)): | 181 | # buffering, write a complete line of output. |
| 61 | 184 | output.append(proc.stdout.readline()) | 182 | for input_line in input_lines: |
| 62 | 183 | proc.stdin.write(input_line + '\n') | ||
| 63 | 184 | output_lines.append(proc.stdout.readline().rstrip('\n')) | ||
| 64 | 185 | os.kill(proc.pid, signal.SIGINT) | 185 | os.kill(proc.pid, signal.SIGINT) |
| 65 | 186 | err = proc.stderr.read() | 186 | err = proc.stderr.read() |
| 66 | 187 | # The script produces logging output, but not to stderr. | 187 | # The script produces logging output, but not to stderr. |
| 67 | 188 | self.assertEqual('', err) | 188 | self.assertEqual('', err) |
| 69 | 189 | self.assertEqual('\n'.join(expected) + '\n', ''.join(output)) | 189 | self.assertEqual(expected_lines, output_lines) |
| 70 | 190 | 190 | ||
| 71 | 191 | 191 | ||
| 72 | 192 | def test_suite(): | 192 | def test_suite(): |
| Stuart Bishop (stub) wrote : | # |
On Wed, Jul 22, 2009 at 11:27 AM, Tim Penhey<email address hidden> wrote:
>> + Â Â Â Â self.store = getUtility(
> SLAVE_FLAVOR)
You can spell this 'self.store = ISlaveStore(
nicer and more meaningful.
>> + Â Â Â Â Â Â if prefix in self._cache:
>> + Â Â Â Â Â Â Â Â branch_id, inserted_time = self._cache[prefix]
>> + Â Â Â Â Â Â Â Â if (self._now() < inserted_time +
>> + Â Â Â Â Â Â Â Â Â Â config.
>> + Â Â Â Â Â Â Â Â Â Â trailing = location[
>> + Â Â Â Â Â Â Â Â Â Â return branch_id, trailing, "HIT"
>> + Â Â Â Â Â Â prefixes.
>> + Â Â Â Â result = self.store.find(
>> + Â Â Â Â Â Â Branch,
>> + Â Â Â Â Â Â Branch.
>> + Â Â Â Â try:
>> + Â Â Â Â Â Â branch_id, unique_name = result.values(
>> + Â Â Â Â Â Â Â Â Branch.id, Branch.
>> + Â Â Â Â except StopIteration:
>> + Â Â Â Â Â Â return None, None, "MISS"
How do we know the Branch was retrieved from the database and not the
Storm cache?
I also don't see any commits. Both of these issues can be fixed in the
schema-lazr.conf - set the isolation level to autocommit and the
storm_cache_size to 0 for this component.
> How about:
>
> Â Â Â result = self.store.find(
> Â Â Â Â Â (Branch.id, Branch.
> Â Â Â Â Â Branch.
>
> That way result should be None if not found, and a tuple of id and unique_name
> if it was found.
>
> That'll make this code a little cleaner.
Shouldn't that be .any()? .one() will explode if multiple rows matched.
--
Stuart Bishop <email address hidden>
http://
| Michael Hudson-Doyle (mwhudson) wrote : | # |
Hi Stuart, thanks for taking a look!
Stuart Bishop wrote:
> On Wed, Jul 22, 2009 at 11:27 AM, Tim Penhey<email address hidden> wrote:
>>> + self.store = getUtility(
>> SLAVE_FLAVOR)
>
> You can spell this 'self.store = ISlaveStore(
> nicer and more meaningful.
Ah, that is nicer.
>>> + if prefix in self._cache:
>>> + branch_id, inserted_time = self._cache[prefix]
>>> + if (self._now() < inserted_time +
>>> + config.
>>> + trailing = location[
>>> + return branch_id, trailing, "HIT"
>>> + prefixes.
>>> + result = self.store.find(
>>> + Branch,
>>> + Branch.
>>> + try:
>>> + branch_id, unique_name = result.values(
>>> + Branch.id, Branch.
>>> + except StopIteration:
>>> + return None, None, "MISS"
>
> How do we know the Branch was retrieved from the database and not the
> Storm cache?
We don't. Is there some way I could test for this?
> I also don't see any commits. Both of these issues can be fixed in the
> schema-lazr.conf - set the isolation level to autocommit and the
> storm_cache_size to 0 for this component.
Why is autocommit better than read committed, given that we don't write
to the database? Serializable would be very very wrong....
I also admit that I don't know what I have to change in schema-lazr.conf
to change the cache size -- what do you mean by 'component'?
>> How about:
>>
>> result = self.store.find(
>> (Branch.id, Branch.
>> Branch.
>>
>> That way result should be None if not found, and a tuple of id and unique_name
>> if it was found.
>>
>> That'll make this code a little cleaner.
>
> Shouldn't that be .any()? .one() will explode if multiple rows matched.
We want it to explode if multiple rows match.
Cheers,
mwh
| Michael Hudson-Doyle (mwhudson) wrote : | # |
Michael Hudson wrote:
>>>> + if prefix in self._cache:
>>>> + branch_id, inserted_time = self._cache[prefix]
>>>> + if (self._now() < inserted_time +
>>>> + config.
>>>> + trailing = location[
>>>> + return branch_id, trailing, "HIT"
>>>> + prefixes.
>>>> + result = self.store.find(
>>>> + Branch,
>>>> + Branch.
>>>> + try:
>>>> + branch_id, unique_name = result.values(
>>>> + Branch.id, Branch.
>>>> + except StopIteration:
>>>> + return None, None, "MISS"
>> How do we know the Branch was retrieved from the database and not the
>> Storm cache?
>
> We don't. Is there some way I could test for this?
Actually we do: we never retrieve a whole branch object from the
database, so there's nothing for storm to cache.
Cheers,
mwh
Preview Diff
| 1 | === modified file 'database/schema/security.cfg' | |||
| 2 | --- database/schema/security.cfg 2009-07-19 04:41:14 +0000 | |||
| 3 | +++ database/schema/security.cfg 2009-07-22 03:37:42 +0000 | |||
| 4 | @@ -1699,3 +1699,7 @@ | |||
| 5 | 1699 | public.person = SELECT | 1699 | public.person = SELECT |
| 6 | 1700 | public.teammembership = SELECT | 1700 | public.teammembership = SELECT |
| 7 | 1701 | public.teamparticipation = SELECT | 1701 | public.teamparticipation = SELECT |
| 8 | 1702 | |||
| 9 | 1703 | [branch-rewrite] | ||
| 10 | 1704 | type=user | ||
| 11 | 1705 | public.branch = SELECT | ||
| 12 | 1702 | 1706 | ||
| 13 | === modified file 'lib/canonical/config/schema-lazr.conf' | |||
| 14 | --- lib/canonical/config/schema-lazr.conf 2009-07-18 01:03:09 +0000 | |||
| 15 | +++ lib/canonical/config/schema-lazr.conf 2009-07-22 03:37:42 +0000 | |||
| 16 | @@ -365,6 +365,12 @@ | |||
| 17 | 365 | # save ourselves some load by directing them all to a HTTP url. | 365 | # save ourselves some load by directing them all to a HTTP url. |
| 18 | 366 | hot_products: | 366 | hot_products: |
| 19 | 367 | 367 | ||
| 20 | 368 | # Branch rewrite cache lifetime. | ||
| 21 | 369 | # | ||
| 22 | 370 | # How long, in seconds, to cache results of the branch path -> id | ||
| 23 | 371 | # mapping done by branch-rewrite.py for. | ||
| 24 | 372 | branch_rewrite_cache_lifetime: 10 | ||
| 25 | 373 | |||
| 26 | 368 | 374 | ||
| 27 | 369 | [codeimport] | 375 | [codeimport] |
| 28 | 370 | # Where the Bazaar imports are stored. | 376 | # Where the Bazaar imports are stored. |
| 29 | 371 | 377 | ||
| 30 | === modified file 'lib/canonical/launchpad/scripts/logger.py' | |||
| 31 | --- lib/canonical/launchpad/scripts/logger.py 2009-06-25 05:30:52 +0000 | |||
| 32 | +++ lib/canonical/launchpad/scripts/logger.py 2009-07-22 02:16:56 +0000 | |||
| 33 | @@ -113,8 +113,8 @@ | |||
| 34 | 113 | def __init__(self): | 113 | def __init__(self): |
| 35 | 114 | self.buffer = StringIO() | 114 | self.buffer = StringIO() |
| 36 | 115 | 115 | ||
| 39 | 116 | def message(self, prefix, *stuff, **kw): | 116 | def message(self, prefix, msg, *stuff, **kw): |
| 40 | 117 | self.buffer.write('%s: %s\n' % (prefix, ' '.join(stuff))) | 117 | self.buffer.write('%s: %s\n' % (prefix, msg % stuff)) |
| 41 | 118 | 118 | ||
| 42 | 119 | if 'exc_info' in kw: | 119 | if 'exc_info' in kw: |
| 43 | 120 | exception = traceback.format_exception(*sys.exc_info()) | 120 | exception = traceback.format_exception(*sys.exc_info()) |
| 44 | 121 | 121 | ||
| 45 | === modified file 'lib/lp/codehosting/rewrite.py' | |||
| 46 | --- lib/lp/codehosting/rewrite.py 2009-07-20 21:32:15 +0000 | |||
| 47 | +++ lib/lp/codehosting/rewrite.py 2009-07-22 03:24:19 +0000 | |||
| 48 | @@ -5,38 +5,71 @@ | |||
| 49 | 5 | """ | 5 | """ |
| 50 | 6 | 6 | ||
| 51 | 7 | import time | 7 | import time |
| 52 | 8 | import xmlrpclib | ||
| 53 | 9 | 8 | ||
| 54 | 10 | from bzrlib import urlutils | 9 | from bzrlib import urlutils |
| 55 | 11 | 10 | ||
| 58 | 12 | from lp.codehosting.vfs import ( | 11 | from canonical.launchpad.webapp.interfaces import ( |
| 59 | 13 | branch_id_to_path, BranchFileSystemClient) | 12 | IStoreSelector, MAIN_STORE, SLAVE_FLAVOR) |
| 60 | 13 | from zope.component import getUtility | ||
| 61 | 14 | from lp.code.model.branch import Branch | ||
| 62 | 15 | from lp.codehosting.vfs import branch_id_to_path | ||
| 63 | 16 | |||
| 64 | 14 | from canonical.config import config | 17 | from canonical.config import config |
| 65 | 15 | from lp.code.interfaces.codehosting import ( | ||
| 66 | 16 | BRANCH_TRANSPORT, LAUNCHPAD_ANONYMOUS) | ||
| 67 | 17 | from canonical.launchpad.xmlrpc import faults | ||
| 68 | 18 | from canonical.twistedsupport import extract_result | ||
| 69 | 19 | 18 | ||
| 70 | 20 | __all__ = ['BranchRewriter'] | 19 | __all__ = ['BranchRewriter'] |
| 71 | 21 | 20 | ||
| 72 | 22 | 21 | ||
| 73 | 23 | class BranchRewriter: | 22 | class BranchRewriter: |
| 74 | 24 | 23 | ||
| 76 | 25 | def __init__(self, logger, proxy): | 24 | def __init__(self, logger, _now=None): |
| 77 | 26 | """ | 25 | """ |
| 78 | 27 | 26 | ||
| 79 | 28 | :param logger: Logger than messages about what the rewriter is doing | 27 | :param logger: Logger than messages about what the rewriter is doing |
| 80 | 29 | will be sent to. | 28 | will be sent to. |
| 81 | 30 | :param proxy: A blocking proxy for a branchfilesystem endpoint. | 29 | :param proxy: A blocking proxy for a branchfilesystem endpoint. |
| 82 | 31 | """ | 30 | """ |
| 83 | 31 | if _now is None: | ||
| 84 | 32 | self._now = time.time | ||
| 85 | 33 | else: | ||
| 86 | 34 | self._now = _now | ||
| 87 | 32 | self.logger = logger | 35 | self.logger = logger |
| 89 | 33 | self.client = BranchFileSystemClient(proxy, LAUNCHPAD_ANONYMOUS, 10.0) | 36 | self.store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR) |
| 90 | 37 | self._cache = {} | ||
| 91 | 34 | 38 | ||
| 92 | 35 | def _codebrowse_url(self, path): | 39 | def _codebrowse_url(self, path): |
| 93 | 36 | return urlutils.join( | 40 | return urlutils.join( |
| 94 | 37 | config.codehosting.internal_codebrowse_root, | 41 | config.codehosting.internal_codebrowse_root, |
| 95 | 38 | path) | 42 | path) |
| 96 | 39 | 43 | ||
| 97 | 44 | def _getBranchIdAndTrailingPath(self, location): | ||
| 98 | 45 | """Return the branch id and trailing path for 'location'. | ||
| 99 | 46 | |||
| 100 | 47 | In addition this method returns whether the answer can from the cache | ||
| 101 | 48 | or from the database. | ||
| 102 | 49 | """ | ||
| 103 | 50 | parts = location[1:].split('/') | ||
| 104 | 51 | prefixes = [] | ||
| 105 | 52 | for i in range(1, len(parts) + 1): | ||
| 106 | 53 | prefix = '/'.join(parts[:i]) | ||
| 107 | 54 | if prefix in self._cache: | ||
| 108 | 55 | branch_id, inserted_time = self._cache[prefix] | ||
| 109 | 56 | if (self._now() < inserted_time + | ||
| 110 | 57 | config.codehosting.branch_rewrite_cache_lifetime): | ||
| 111 | 58 | trailing = location[len(prefix) + 1:] | ||
| 112 | 59 | return branch_id, trailing, "HIT" | ||
| 113 | 60 | prefixes.append(prefix) | ||
| 114 | 61 | result = self.store.find( | ||
| 115 | 62 | Branch, | ||
| 116 | 63 | Branch.unique_name.is_in(prefixes), Branch.private == False) | ||
| 117 | 64 | try: | ||
| 118 | 65 | branch_id, unique_name = result.values( | ||
| 119 | 66 | Branch.id, Branch.unique_name).next() | ||
| 120 | 67 | except StopIteration: | ||
| 121 | 68 | return None, None, "MISS" | ||
| 122 | 69 | self._cache[unique_name] = (branch_id, self._now()) | ||
| 123 | 70 | trailing = location[len(unique_name) + 1:] | ||
| 124 | 71 | return branch_id, trailing, "MISS" | ||
| 125 | 72 | |||
| 126 | 40 | def rewriteLine(self, resource_location): | 73 | def rewriteLine(self, resource_location): |
| 127 | 41 | """Rewrite 'resource_location' to a more concrete location. | 74 | """Rewrite 'resource_location' to a more concrete location. |
| 128 | 42 | 75 | ||
| 129 | @@ -65,35 +98,26 @@ | |||
| 130 | 65 | Other errors are allowed to propagate, on the assumption that the | 98 | Other errors are allowed to propagate, on the assumption that the |
| 131 | 66 | caller will catch and log them. | 99 | caller will catch and log them. |
| 132 | 67 | """ | 100 | """ |
| 133 | 68 | T = time.time() | ||
| 134 | 69 | # Codebrowse generates references to its images and stylesheets | 101 | # Codebrowse generates references to its images and stylesheets |
| 135 | 70 | # starting with "/static", so pass them on unthinkingly. | 102 | # starting with "/static", so pass them on unthinkingly. |
| 136 | 103 | T = time.time() | ||
| 137 | 104 | cached = None | ||
| 138 | 71 | if resource_location.startswith('/static/'): | 105 | if resource_location.startswith('/static/'): |
| 161 | 72 | return self._codebrowse_url(resource_location) | 106 | r = self._codebrowse_url(resource_location) |
| 162 | 73 | trailingSlash = resource_location.endswith('/') | 107 | cached = 'N/A' |
| 163 | 74 | deferred = self.client.translatePath(resource_location) | 108 | else: |
| 164 | 75 | try: | 109 | branch_id, trailing, cached = self._getBranchIdAndTrailingPath( |
| 165 | 76 | transport_type, info, trailing = extract_result(deferred) | 110 | resource_location) |
| 166 | 77 | except xmlrpclib.Fault, f: | 111 | if branch_id is None: |
| 145 | 78 | if (faults.check_fault(f, faults.PathTranslationError) | ||
| 146 | 79 | or faults.check_fault(f, faults.PermissionDenied)): | ||
| 147 | 80 | # In this situation, we'd *like* to generate a 404 | ||
| 148 | 81 | # (PathTranslationError) or 301 (PermissionDenied) error. But | ||
| 149 | 82 | # we can't, so we just forward on to codebrowse which can. | ||
| 150 | 83 | return self._codebrowse_url(resource_location) | ||
| 151 | 84 | else: | ||
| 152 | 85 | raise | ||
| 153 | 86 | if transport_type == BRANCH_TRANSPORT: | ||
| 154 | 87 | if trailing.startswith('.bzr'): | ||
| 155 | 88 | r = urlutils.join( | ||
| 156 | 89 | config.codehosting.internal_branch_by_id_root, | ||
| 157 | 90 | branch_id_to_path(info['id']), trailing) | ||
| 158 | 91 | if trailingSlash: | ||
| 159 | 92 | r += '/' | ||
| 160 | 93 | else: | ||
| 167 | 94 | r = self._codebrowse_url(resource_location) | 112 | r = self._codebrowse_url(resource_location) |
| 173 | 95 | self.logger.info( | 113 | else: |
| 174 | 96 | "%r -> %r (%fs)", resource_location, r, time.time() - T) | 114 | if trailing.startswith('/.bzr'): |
| 175 | 97 | return r | 115 | r = urlutils.join( |
| 176 | 98 | else: | 116 | config.codehosting.internal_branch_by_id_root, |
| 177 | 99 | return "NULL" | 117 | branch_id_to_path(branch_id), trailing[1:]) |
| 178 | 118 | else: | ||
| 179 | 119 | r = self._codebrowse_url(resource_location) | ||
| 180 | 120 | self.logger.info( | ||
| 181 | 121 | "%r -> %r (%fs, cache: %s)", | ||
| 182 | 122 | resource_location, r, time.time() - T, cached) | ||
| 183 | 123 | return r | ||
| 184 | 100 | 124 | ||
| 185 | === modified file 'lib/lp/codehosting/tests/test_rewrite.py' | |||
| 186 | --- lib/lp/codehosting/tests/test_rewrite.py 2009-07-17 00:26:05 +0000 | |||
| 187 | +++ lib/lp/codehosting/tests/test_rewrite.py 2009-07-22 03:23:28 +0000 | |||
| 188 | @@ -5,68 +5,91 @@ | |||
| 189 | 5 | 5 | ||
| 190 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
| 191 | 7 | 7 | ||
| 192 | 8 | import re | ||
| 193 | 8 | import os | 9 | import os |
| 194 | 9 | import signal | 10 | import signal |
| 195 | 10 | import subprocess | 11 | import subprocess |
| 196 | 11 | import unittest | 12 | import unittest |
| 197 | 12 | 13 | ||
| 198 | 14 | import transaction | ||
| 199 | 15 | |||
| 200 | 16 | from zope.security.proxy import removeSecurityProxy | ||
| 201 | 17 | |||
| 202 | 13 | from lp.codehosting.vfs import branch_id_to_path | 18 | from lp.codehosting.vfs import branch_id_to_path |
| 203 | 14 | from lp.codehosting.inmemory import InMemoryFrontend, XMLRPCWrapper | ||
| 204 | 15 | from lp.codehosting.rewrite import BranchRewriter | 19 | from lp.codehosting.rewrite import BranchRewriter |
| 205 | 16 | from canonical.config import config | 20 | from canonical.config import config |
| 212 | 17 | from lp.testing import TestCase, TestCaseWithFactory | 21 | from lp.testing import FakeTime, TestCaseWithFactory |
| 213 | 18 | from canonical.launchpad.scripts import QuietFakeLogger | 22 | from canonical.launchpad.scripts import BufferLogger |
| 214 | 19 | from canonical.testing.layers import ZopelessAppServerLayer | 23 | from canonical.testing.layers import DatabaseFunctionalLayer |
| 215 | 20 | 24 | ||
| 216 | 21 | 25 | ||
| 217 | 22 | class TestBranchRewriter(TestCase): | 26 | class TestBranchRewriter(TestCaseWithFactory): |
| 218 | 27 | |||
| 219 | 28 | layer = DatabaseFunctionalLayer | ||
| 220 | 23 | 29 | ||
| 221 | 24 | def setUp(self): | 30 | def setUp(self): |
| 225 | 25 | frontend = InMemoryFrontend() | 31 | TestCaseWithFactory.setUp(self) |
| 226 | 26 | self._branchfs = frontend.getFilesystemEndpoint() | 32 | self.fake_time = FakeTime(0) |
| 224 | 27 | self.factory = frontend.getLaunchpadObjectFactory() | ||
| 227 | 28 | 33 | ||
| 228 | 29 | def makeRewriter(self): | 34 | def makeRewriter(self): |
| 233 | 30 | return BranchRewriter( | 35 | return BranchRewriter(BufferLogger(), self.fake_time.now) |
| 234 | 31 | QuietFakeLogger(), XMLRPCWrapper(self._branchfs)) | 36 | |
| 235 | 32 | 37 | def getLoggerOutput(self, rewriter): | |
| 236 | 33 | def test_translateLine_found_dot_bzr(self): | 38 | return rewriter.logger.buffer.getvalue() |
| 237 | 39 | |||
| 238 | 40 | def test_rewriteLine_found_dot_bzr(self): | ||
| 239 | 34 | # Requests for /$branch_name/.bzr/... are redirected to where the | 41 | # Requests for /$branch_name/.bzr/... are redirected to where the |
| 240 | 35 | # branches are served from by ID. | 42 | # branches are served from by ID. |
| 241 | 36 | rewriter = self.makeRewriter() | 43 | rewriter = self.makeRewriter() |
| 245 | 37 | branch = self.factory.makeAnyBranch() | 44 | branches = [ |
| 246 | 38 | line = rewriter.rewriteLine("/%s/.bzr/README" % branch.unique_name) | 45 | self.factory.makeProductBranch(), |
| 247 | 39 | self.assertEqual( | 46 | self.factory.makePersonalBranch(), |
| 248 | 47 | self.factory.makePackageBranch()] | ||
| 249 | 48 | transaction.commit() | ||
| 250 | 49 | output = [ | ||
| 251 | 50 | rewriter.rewriteLine("/%s/.bzr/README" % branch.unique_name) | ||
| 252 | 51 | for branch in branches] | ||
| 253 | 52 | expected = [ | ||
| 254 | 40 | 'file:///var/tmp/bzrsync/%s/.bzr/README' | 53 | 'file:///var/tmp/bzrsync/%s/.bzr/README' |
| 257 | 41 | % branch_id_to_path(branch.id), | 54 | % branch_id_to_path(branch.id) |
| 258 | 42 | line) | 55 | for branch in branches] |
| 259 | 56 | self.assertEqual(expected, output) | ||
| 260 | 43 | 57 | ||
| 262 | 44 | def test_translateLine_found_not_dot_bzr(self): | 58 | def test_rewriteLine_found_not_dot_bzr(self): |
| 263 | 45 | # Requests for /$branch_name/... that are not to .bzr directories are | 59 | # Requests for /$branch_name/... that are not to .bzr directories are |
| 264 | 46 | # redirected to codebrowse. | 60 | # redirected to codebrowse. |
| 265 | 47 | rewriter = self.makeRewriter() | 61 | rewriter = self.makeRewriter() |
| 271 | 48 | branch = self.factory.makeAnyBranch() | 62 | branches = [ |
| 272 | 49 | output = rewriter.rewriteLine("/%s/changes" % branch.unique_name) | 63 | self.factory.makeProductBranch(), |
| 273 | 50 | self.assertEqual( | 64 | self.factory.makePersonalBranch(), |
| 274 | 51 | 'http://localhost:8080/%s/changes' % branch.unique_name, | 65 | self.factory.makePackageBranch()] |
| 275 | 52 | output) | 66 | transaction.commit() |
| 276 | 67 | output = [ | ||
| 277 | 68 | rewriter.rewriteLine("/%s/changes" % branch.unique_name) | ||
| 278 | 69 | for branch in branches] | ||
| 279 | 70 | expected = [ | ||
| 280 | 71 | 'http://localhost:8080/%s/changes' % branch.unique_name | ||
| 281 | 72 | for branch in branches] | ||
| 282 | 73 | self.assertEqual(expected, output) | ||
| 283 | 53 | 74 | ||
| 285 | 54 | def test_translateLine_private(self): | 75 | def test_rewriteLine_private(self): |
| 286 | 55 | # All requests for /$branch_name/... for private branches are | 76 | # All requests for /$branch_name/... for private branches are |
| 287 | 56 | # rewritten to codebrowse, which will then redirect them to https and | 77 | # rewritten to codebrowse, which will then redirect them to https and |
| 288 | 57 | # handle them there. | 78 | # handle them there. |
| 289 | 58 | rewriter = self.makeRewriter() | 79 | rewriter = self.makeRewriter() |
| 298 | 59 | branch = self.factory.makeBranch(private=True) | 80 | branch = self.factory.makeAnyBranch(private=True) |
| 299 | 60 | output = rewriter.rewriteLine("/%s/changes" % branch.unique_name) | 81 | unique_name = removeSecurityProxy(branch).unique_name |
| 300 | 61 | self.assertEqual( | 82 | transaction.commit() |
| 301 | 62 | 'http://localhost:8080/%s/changes' % branch.unique_name, | 83 | output = [ |
| 302 | 63 | output) | 84 | rewriter.rewriteLine("/%s/changes" % unique_name), |
| 303 | 64 | output = rewriter.rewriteLine("/%s/.bzr" % branch.unique_name) | 85 | rewriter.rewriteLine("/%s/.bzr" % unique_name) |
| 304 | 65 | self.assertEqual( | 86 | ] |
| 305 | 66 | 'http://localhost:8080/%s/.bzr' % branch.unique_name, | 87 | self.assertEqual( |
| 306 | 88 | ['http://localhost:8080/%s/changes' % unique_name, | ||
| 307 | 89 | 'http://localhost:8080/%s/.bzr' % unique_name], | ||
| 308 | 67 | output) | 90 | output) |
| 309 | 68 | 91 | ||
| 311 | 69 | def test_translateLine_static(self): | 92 | def test_rewriteLine_static(self): |
| 312 | 70 | # Requests to /static are rewritten to codebrowse urls. | 93 | # Requests to /static are rewritten to codebrowse urls. |
| 313 | 71 | rewriter = self.makeRewriter() | 94 | rewriter = self.makeRewriter() |
| 314 | 72 | output = rewriter.rewriteLine("/static/foo") | 95 | output = rewriter.rewriteLine("/static/foo") |
| 315 | @@ -74,7 +97,7 @@ | |||
| 316 | 74 | 'http://localhost:8080/static/foo', | 97 | 'http://localhost:8080/static/foo', |
| 317 | 75 | output) | 98 | output) |
| 318 | 76 | 99 | ||
| 320 | 77 | def test_translateLine_not_found(self): | 100 | def test_rewriteLine_not_found(self): |
| 321 | 78 | # If the request does not map to a branch, we redirect it to | 101 | # If the request does not map to a branch, we redirect it to |
| 322 | 79 | # codebrowse as it can generate a 404. | 102 | # codebrowse as it can generate a 404. |
| 323 | 80 | rewriter = self.makeRewriter() | 103 | rewriter = self.makeRewriter() |
| 324 | @@ -84,30 +107,86 @@ | |||
| 325 | 84 | 'http://localhost:8080%s' % not_found_path, | 107 | 'http://localhost:8080%s' % not_found_path, |
| 326 | 85 | output) | 108 | output) |
| 327 | 86 | 109 | ||
| 328 | 110 | def test_rewriteLine_logs_cache_miss(self): | ||
| 329 | 111 | # The first request for a branch misses the cache and logs this fact. | ||
| 330 | 112 | rewriter = self.makeRewriter() | ||
| 331 | 113 | branch = self.factory.makeAnyBranch() | ||
| 332 | 114 | transaction.commit() | ||
| 333 | 115 | rewriter.rewriteLine('/' + branch.unique_name + '/.bzr/README') | ||
| 334 | 116 | logging_output = self.getLoggerOutput(rewriter) | ||
| 335 | 117 | self.assertIsNot( | ||
| 336 | 118 | None, | ||
| 337 | 119 | re.match("INFO: .* -> .* (.*s, cache: MISS)", logging_output), | ||
| 338 | 120 | "No miss found in %r" % logging_output) | ||
| 339 | 121 | |||
| 340 | 122 | def test_rewriteLine_logs_cache_hit(self): | ||
| 341 | 123 | # The second request for a branch misses the cache and logs this fact. | ||
| 342 | 124 | rewriter = self.makeRewriter() | ||
| 343 | 125 | branch = self.factory.makeAnyBranch() | ||
| 344 | 126 | transaction.commit() | ||
| 345 | 127 | rewriter.rewriteLine('/' + branch.unique_name + '/.bzr/README') | ||
| 346 | 128 | rewriter.rewriteLine('/' + branch.unique_name + '/.bzr/README') | ||
| 347 | 129 | logging_output_lines = self.getLoggerOutput(rewriter).strip().split('\n') | ||
| 348 | 130 | self.assertEqual(2, len(logging_output_lines)) | ||
| 349 | 131 | self.assertIsNot( | ||
| 350 | 132 | None, | ||
| 351 | 133 | re.match("INFO: .* -> .* (.*s, cache: HIT)", | ||
| 352 | 134 | logging_output_lines[-1]), | ||
| 353 | 135 | "No hit found in %r" % logging_output_lines[-1]) | ||
| 354 | 136 | |||
| 355 | 137 | def test_rewriteLine_cache_expires(self): | ||
| 356 | 138 | # The second request for a branch misses the cache and logs this fact. | ||
| 357 | 139 | rewriter = self.makeRewriter() | ||
| 358 | 140 | branch = self.factory.makeAnyBranch() | ||
| 359 | 141 | transaction.commit() | ||
| 360 | 142 | rewriter.rewriteLine('/' + branch.unique_name + '/.bzr/README') | ||
| 361 | 143 | self.fake_time.advance( | ||
| 362 | 144 | config.codehosting.branch_rewrite_cache_lifetime + 1) | ||
| 363 | 145 | rewriter.rewriteLine('/' + branch.unique_name + '/.bzr/README') | ||
| 364 | 146 | logging_output_lines = self.getLoggerOutput(rewriter).strip().split('\n') | ||
| 365 | 147 | self.assertEqual(2, len(logging_output_lines)) | ||
| 366 | 148 | self.assertIsNot( | ||
| 367 | 149 | None, | ||
| 368 | 150 | re.match("INFO: .* -> .* (.*s, cache: MISS)", | ||
| 369 | 151 | logging_output_lines[-1]), | ||
| 370 | 152 | "No miss found in %r" % logging_output_lines[-1]) | ||
| 371 | 153 | |||
| 372 | 87 | 154 | ||
| 373 | 88 | class TestBranchRewriterScript(TestCaseWithFactory): | 155 | class TestBranchRewriterScript(TestCaseWithFactory): |
| 374 | 89 | 156 | ||
| 376 | 90 | layer = ZopelessAppServerLayer | 157 | layer = DatabaseFunctionalLayer |
| 377 | 91 | 158 | ||
| 378 | 92 | def test_script(self): | 159 | def test_script(self): |
| 385 | 93 | branch = self.factory.makeAnyBranch() | 160 | branches = [ |
| 386 | 94 | input = "/%s/.bzr/README\n" % branch.unique_name | 161 | self.factory.makeProductBranch(), |
| 387 | 95 | expected = ( | 162 | self.factory.makePersonalBranch(), |
| 388 | 96 | "file:///var/tmp/bzrsync/%s/.bzr/README\n" | 163 | self.factory.makePackageBranch()] |
| 389 | 97 | % branch_id_to_path(branch.id)) | 164 | input = [ |
| 390 | 98 | self.layer.txn.commit() | 165 | "/%s/.bzr/README" % branch.unique_name |
| 391 | 166 | for branch in branches] + [ | ||
| 392 | 167 | "/%s/changes" % branch.unique_name | ||
| 393 | 168 | for branch in branches] | ||
| 394 | 169 | expected = [ | ||
| 395 | 170 | 'file:///var/tmp/bzrsync/%s/.bzr/README' | ||
| 396 | 171 | % branch_id_to_path(branch.id) | ||
| 397 | 172 | for branch in branches] + [ | ||
| 398 | 173 | 'http://localhost:8080/%s/changes' % branch.unique_name | ||
| 399 | 174 | for branch in branches] | ||
| 400 | 175 | transaction.commit() | ||
| 401 | 99 | script_file = os.path.join( | 176 | script_file = os.path.join( |
| 402 | 100 | config.root, 'scripts', 'branch-rewrite.py') | 177 | config.root, 'scripts', 'branch-rewrite.py') |
| 403 | 101 | proc = subprocess.Popen( | 178 | proc = subprocess.Popen( |
| 404 | 102 | [script_file], stdin=subprocess.PIPE, stdout=subprocess.PIPE, | 179 | [script_file], stdin=subprocess.PIPE, stdout=subprocess.PIPE, |
| 405 | 103 | stderr=subprocess.PIPE, bufsize=0) | 180 | stderr=subprocess.PIPE, bufsize=0) |
| 408 | 104 | proc.stdin.write(input) | 181 | proc.stdin.write('\n'.join(input) + '\n') |
| 409 | 105 | output = proc.stdout.readline() | 182 | output = [] |
| 410 | 183 | for i in range(len(input)): | ||
| 411 | 184 | output.append(proc.stdout.readline()) | ||
| 412 | 106 | os.kill(proc.pid, signal.SIGINT) | 185 | os.kill(proc.pid, signal.SIGINT) |
| 413 | 107 | err = proc.stderr.read() | 186 | err = proc.stderr.read() |
| 414 | 108 | # The script produces logging output, but not to stderr. | 187 | # The script produces logging output, but not to stderr. |
| 415 | 109 | self.assertEqual('', err) | 188 | self.assertEqual('', err) |
| 417 | 110 | self.assertEqual(expected, output) | 189 | self.assertEqual('\n'.join(expected) + '\n', ''.join(output)) |
| 418 | 111 | 190 | ||
| 419 | 112 | 191 | ||
| 420 | 113 | def test_suite(): | 192 | def test_suite(): |
| 421 | 114 | 193 | ||
| 422 | === modified file 'lib/lp/testing/__init__.py' | |||
| 423 | --- lib/lp/testing/__init__.py 2009-07-17 18:46:25 +0000 | |||
| 424 | +++ lib/lp/testing/__init__.py 2009-07-22 02:26:16 +0000 | |||
| 425 | @@ -224,10 +224,11 @@ | |||
| 426 | 224 | self.assertTrue(expected is observed, | 224 | self.assertTrue(expected is observed, |
| 427 | 225 | "%r is not %r" % (expected, observed)) | 225 | "%r is not %r" % (expected, observed)) |
| 428 | 226 | 226 | ||
| 430 | 227 | def assertIsNot(self, expected, observed): | 227 | def assertIsNot(self, expected, observed, msg=None): |
| 431 | 228 | """Assert that `expected` is not the same object as `observed`.""" | 228 | """Assert that `expected` is not the same object as `observed`.""" |
| 434 | 229 | self.assertTrue(expected is not observed, | 229 | if msg is None: |
| 435 | 230 | "%r is %r" % (expected, observed)) | 230 | msg = "%r is %r" % (expected, observed) |
| 436 | 231 | self.assertTrue(expected is not observed, msg) | ||
| 437 | 231 | 232 | ||
| 438 | 232 | def assertIn(self, needle, haystack): | 233 | def assertIn(self, needle, haystack): |
| 439 | 233 | """Assert that 'needle' is in 'haystack'.""" | 234 | """Assert that 'needle' is in 'haystack'.""" |
| 440 | 234 | 235 | ||
| 441 | === modified file 'scripts/branch-rewrite.py' | |||
| 442 | --- scripts/branch-rewrite.py 2009-06-30 16:56:07 +0000 | |||
| 443 | +++ scripts/branch-rewrite.py 2009-07-22 03:36:19 +0000 | |||
| 444 | @@ -14,10 +14,13 @@ | |||
| 445 | 14 | 14 | ||
| 446 | 15 | import _pythonpath | 15 | import _pythonpath |
| 447 | 16 | 16 | ||
| 448 | 17 | import os | ||
| 449 | 17 | import sys | 18 | import sys |
| 453 | 18 | import xmlrpclib | 19 | |
| 454 | 19 | 20 | # XXX, MichaelHudson, 2009-07-22, bug=402845: This pointless import avoids a | |
| 455 | 20 | from lp.codehosting.vfs import BlockingProxy | 21 | # circular import killing us. |
| 456 | 22 | from canonical.launchpad.database import account | ||
| 457 | 23 | |||
| 458 | 21 | from lp.codehosting.rewrite import BranchRewriter | 24 | from lp.codehosting.rewrite import BranchRewriter |
| 459 | 22 | from canonical.config import config | 25 | from canonical.config import config |
| 460 | 23 | from lp.services.scripts.base import LaunchpadScript | 26 | from lp.services.scripts.base import LaunchpadScript |
| 461 | @@ -25,11 +28,6 @@ | |||
| 462 | 25 | 28 | ||
| 463 | 26 | class BranchRewriteScript(LaunchpadScript): | 29 | class BranchRewriteScript(LaunchpadScript): |
| 464 | 27 | 30 | ||
| 465 | 28 | def __init__(self, name): | ||
| 466 | 29 | LaunchpadScript.__init__(self, name) | ||
| 467 | 30 | proxy = xmlrpclib.ServerProxy(config.codehosting.branchfs_endpoint) | ||
| 468 | 31 | self.rewriter = BranchRewriter(self.logger, BlockingProxy(proxy)) | ||
| 469 | 32 | |||
| 470 | 33 | def add_my_options(self): | 31 | def add_my_options(self): |
| 471 | 34 | """Make the logging go to a file by default. | 32 | """Make the logging go to a file by default. |
| 472 | 35 | 33 | ||
| 473 | @@ -40,26 +38,20 @@ | |||
| 474 | 40 | value from the config. | 38 | value from the config. |
| 475 | 41 | """ | 39 | """ |
| 476 | 42 | log_file_location = config.codehosting.rewrite_script_log_file | 40 | log_file_location = config.codehosting.rewrite_script_log_file |
| 477 | 41 | log_file_directory = os.path.dirname(log_file_location) | ||
| 478 | 42 | if not os.path.isdir(log_file_directory): | ||
| 479 | 43 | os.makedirs(log_file_directory) | ||
| 480 | 43 | self.parser.defaults['log_file'] = log_file_location | 44 | self.parser.defaults['log_file'] = log_file_location |
| 481 | 44 | 45 | ||
| 482 | 45 | def run(self, use_web_security=False, implicit_begin=True, | ||
| 483 | 46 | isolation=None): | ||
| 484 | 47 | """See `LaunchpadScript.run`. | ||
| 485 | 48 | |||
| 486 | 49 | As this script does not need the component architecture or a | ||
| 487 | 50 | connection to the database, we override this method to avoid setting | ||
| 488 | 51 | them up. | ||
| 489 | 52 | """ | ||
| 490 | 53 | self.main() | ||
| 491 | 54 | |||
| 492 | 55 | def main(self): | 46 | def main(self): |
| 493 | 47 | rewriter = BranchRewriter(self.logger) | ||
| 494 | 56 | self.logger.debug("Starting up...") | 48 | self.logger.debug("Starting up...") |
| 495 | 57 | while True: | 49 | while True: |
| 496 | 58 | try: | 50 | try: |
| 497 | 59 | line = sys.stdin.readline() | 51 | line = sys.stdin.readline() |
| 498 | 60 | # Mod-rewrite always gives us a newline terminated string. | 52 | # Mod-rewrite always gives us a newline terminated string. |
| 499 | 61 | if line: | 53 | if line: |
| 501 | 62 | print self.rewriter.rewriteLine(line.strip()) | 54 | print rewriter.rewriteLine(line.strip()) |
| 502 | 63 | else: | 55 | else: |
| 503 | 64 | # Standard input has been closed, so die. | 56 | # Standard input has been closed, so die. |
| 504 | 65 | return | 57 | return |
| 505 | @@ -71,4 +63,4 @@ | |||
| 506 | 71 | 63 | ||
| 507 | 72 | 64 | ||
| 508 | 73 | if __name__ == '__main__': | 65 | if __name__ == '__main__': |
| 510 | 74 | BranchRewriteScript("branch-rewrite").run() | 66 | BranchRewriteScript("branch-rewrite", dbuser='branch-rewrite').run() |
As we've discovered, we need the rewriting done by the branch-rewrite.py script to be much quicker. So this branch implements it via a single database query. This should (a) be much much faster (b) take load off the XML-RPC server.
In addition, it records in the log whether the cache was hit or not.
There is now some duplicated code between lp.codehosting.