Launchpad itself

Merge lp:~mwhudson/launchpad/lp-serve-report-branch-access-bug-532210 into lp:launchpad

  1. lp-serve-report-branch-access-bug-532210
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~mwhudson/launchpad/lp-serve-report-branch-access-bug-532210
Merge into: lp:launchpad
Diff against target: 326 lines (+154/-11)
8 files modified
bzrplugins/lpserve.py (+3/-2)
lib/lp/codehosting/vfs/branchfs.py (+14/-6)
lib/lp/codehosting/vfs/branchfsclient.py (+9/-1)
lib/lp/codehosting/vfs/hooks.py (+25/-0)
lib/lp/codehosting/vfs/tests/test_branchfsclient.py (+37/-1)
lib/lp/codehosting/vfs/tests/test_hooks.py (+63/-0)
setup.py (+2/-1)
versions.cfg (+1/-0)
To merge this branch: bzr merge lp:~mwhudson/launchpad/lp-serve-report-branch-access-bug-532210
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+21907@code.launchpad.net

Commit message

Report the branch being accessed in the lp-serve process' process name

Description of the change

Hi there,

This branch reports which branch an lp-serve accesses in its process title.

I didn't have a pre-imp call for this, and if I did I probably wouldn't be so worried that the code in lp.codehosting.vfs.hooks isn't in a sensibly named file.

Cheers,
mwh

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Looks good. I hope it works :-)

review: Approve
Revision history for this message
Jonathan Lange (jml) wrote :

Cool! At first I thought this was about logging branch access to a file, but it's still great news.

This might be a silly question, but have you run this locally?

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

On 23/03/10 23:29, Jonathan Lange wrote:
> Cool! At first I thought this was about logging branch access to a file, but it's still great news.

Yeah, we should probably do that, but multiprocess synchronization argh.
  What I'd like is to have bzr lp-serve processes log to ~/.bzr.log.$pid
rather than all smearing into ~/.bzr.log but that was annoying last time
I looked into it.

> This might be a silly question, but have you run this locally?

Yes I have. It seems to work :)

Cheers,
mwh

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrplugins/lpserve.py'
2--- bzrplugins/lpserve.py 2010-03-16 19:01:26 +0000
3+++ bzrplugins/lpserve.py 2010-03-24 00:47:00 +0000
4@@ -88,13 +88,14 @@
5 def run(self, user_id, port=None, upload_directory=None,
6 mirror_directory=None, branchfs_endpoint_url=None, inet=False):
7 from lp.codehosting.bzrutils import install_oops_handler
8- from lp.codehosting.vfs import get_lp_server
9+ from lp.codehosting.vfs import get_lp_server, hooks
10 install_oops_handler(user_id)
11 four_gig = int(4e9)
12 resource.setrlimit(resource.RLIMIT_AS, (four_gig, four_gig))
13+ seen_new_branch = hooks.SetProcTitleHook()
14 lp_server = get_lp_server(
15 int(user_id), branchfs_endpoint_url,
16- upload_directory, mirror_directory)
17+ upload_directory, mirror_directory, seen_new_branch.seen)
18 lp_server.start_server()
19
20 old_lockdir_timeout = lockdir._DEFAULT_TIMEOUT_SECONDS
21
22=== modified file 'lib/lp/codehosting/vfs/branchfs.py'
23--- lib/lp/codehosting/vfs/branchfs.py 2010-02-14 23:02:37 +0000
24+++ lib/lp/codehosting/vfs/branchfs.py 2010-03-24 00:47:00 +0000
25@@ -368,16 +368,20 @@
26 path on that transport.
27 """
28
29- def __init__(self, scheme, authserver, user_id):
30+ def __init__(self, scheme, authserver, user_id,
31+ seen_new_branch_hook=None):
32 """Construct a LaunchpadServer.
33
34 :param scheme: The URL scheme to use.
35 :param authserver: An XML-RPC client that implements callRemote.
36 :param user_id: The database ID for the user who is accessing
37 branches.
38+ :param seen_new_branch_hook: A callable that will be called once for
39+ each branch accessed via this server.
40 """
41 AsyncVirtualServer.__init__(self, scheme)
42- self._authserver = BranchFileSystemClient(authserver, user_id)
43+ self._authserver = BranchFileSystemClient(
44+ authserver, user_id, seen_new_branch_hook=seen_new_branch_hook)
45 self._is_start_server = False
46
47 def translateVirtualPath(self, virtual_url_fragment):
48@@ -556,7 +560,7 @@
49 asyncTransportFactory = AsyncLaunchpadTransport
50
51 def __init__(self, authserver, user_id, hosted_transport,
52- mirror_transport):
53+ mirror_transport, seen_new_branch_hook=None):
54 """Construct a `LaunchpadServer`.
55
56 See `_BaseLaunchpadServer` for more information.
57@@ -575,9 +579,12 @@
58 :param mirror_transport: A Bazaar `Transport` that points to the
59 "mirrored" area of Launchpad. See module docstring for more
60 information.
61+ :param seen_new_branch_hook: A callable that will be called once for
62+ each branch accessed via this server.
63 """
64 scheme = 'lp-%d:///' % id(self)
65- super(LaunchpadServer, self).__init__(scheme, authserver, user_id)
66+ super(LaunchpadServer, self).__init__(
67+ scheme, authserver, user_id, seen_new_branch_hook)
68 mirror_transport = get_readonly_transport(mirror_transport)
69 self._transport_dispatch = TransportDispatch(
70 hosted_transport, mirror_transport)
71@@ -641,7 +648,7 @@
72
73
74 def get_lp_server(user_id, branchfs_endpoint_url=None, hosted_directory=None,
75- mirror_directory=None):
76+ mirror_directory=None, seen_new_branch_hook=None):
77 """Create a Launchpad server.
78
79 :param user_id: A unique database ID of the user whose branches are
80@@ -649,6 +656,7 @@
81 :param branchfs_endpoint_url: URL for the branch file system end-point.
82 :param hosted_directory: Where the branches are uploaded to.
83 :param mirror_directory: Where all Launchpad branches are mirrored.
84+ :param seen_new_branch_hook:
85 :return: A `LaunchpadServer`.
86 """
87 # Get the defaults from the config.
88@@ -667,7 +675,7 @@
89 mirror_transport = get_chrooted_transport(mirror_url)
90 lp_server = LaunchpadServer(
91 BlockingProxy(branchfs_client), user_id,
92- hosted_transport, mirror_transport)
93+ hosted_transport, mirror_transport, seen_new_branch_hook)
94 return lp_server
95
96
97
98=== modified file 'lib/lp/codehosting/vfs/branchfsclient.py'
99--- lib/lp/codehosting/vfs/branchfsclient.py 2009-06-25 04:06:00 +0000
100+++ lib/lp/codehosting/vfs/branchfsclient.py 2010-03-24 00:47:00 +0000
101@@ -46,18 +46,24 @@
102 """
103
104 def __init__(self, branchfs_endpoint, user_id, expiry_time=None,
105- _now=time.time):
106+ seen_new_branch_hook=None, _now=time.time):
107 """Construct a caching branchfs_endpoint.
108
109 :param branchfs_endpoint: An XML-RPC proxy that implements callRemote.
110 :param user_id: The database ID of the user who will be making these
111 requests. An integer.
112+ :param expiry_time: If supplied, only cache the results of
113+ translatePath for this many seconds. If not supplied, cache the
114+ results of translatePath for as long as this instance exists.
115+ :param seen_new_branch_hook: A callable that will be called with the
116+ unique_name of each new branch that is accessed.
117 """
118 self._branchfs_endpoint = branchfs_endpoint
119 self._cache = {}
120 self._user_id = user_id
121 self.expiry_time = expiry_time
122 self._now = _now
123+ self.seen_new_branch_hook = seen_new_branch_hook
124
125 def _getMatchedPart(self, path, transport_tuple):
126 """Return the part of 'path' that the endpoint actually matched."""
127@@ -77,6 +83,8 @@
128 (transport_type, data, trailing_path) = transport_tuple
129 matched_part = self._getMatchedPart(path, transport_tuple)
130 if transport_type == BRANCH_TRANSPORT:
131+ if self.seen_new_branch_hook:
132+ self.seen_new_branch_hook(matched_part.strip('/'))
133 self._cache[matched_part] = (transport_type, data, self._now())
134 return transport_tuple
135
136
137=== added file 'lib/lp/codehosting/vfs/hooks.py'
138--- lib/lp/codehosting/vfs/hooks.py 1970-01-01 00:00:00 +0000
139+++ lib/lp/codehosting/vfs/hooks.py 2010-03-24 00:47:00 +0000
140@@ -0,0 +1,25 @@
141+# Copyright 2010 Canonical Ltd. This software is licensed under the
142+# GNU Affero General Public License version 3 (see the file LICENSE).
143+
144+"""Implementations for the `seen_new_branch_hook` of `BranchFileSystemClient`.
145+"""
146+
147+__metaclass__ = type
148+__all__ = ['SetProcTitleHook']
149+
150+import setproctitle
151+
152+
153+class SetProcTitleHook:
154+ """Use seen() as the hook to report branch access in ps(1) output."""
155+
156+ def __init__(self, setproctitle_mod=None):
157+ if setproctitle_mod is None:
158+ setproctitle_mod = setproctitle
159+ self.setproctitle_mod = setproctitle_mod
160+ self.basename = setproctitle_mod.getproctitle()
161+
162+ def seen(self, branch_url):
163+ branch_url = branch_url.strip('/')
164+ self.setproctitle_mod.setproctitle(
165+ self.basename + ' BRANCH:' + branch_url)
166
167=== modified file 'lib/lp/codehosting/vfs/tests/test_branchfsclient.py'
168--- lib/lp/codehosting/vfs/tests/test_branchfsclient.py 2009-06-25 04:06:00 +0000
169+++ lib/lp/codehosting/vfs/tests/test_branchfsclient.py 2010-03-24 00:47:00 +0000
170@@ -36,13 +36,14 @@
171 """
172 self.fake_time.advance(amount)
173
174- def makeClient(self, expiry_time=None):
175+ def makeClient(self, expiry_time=None, seen_new_branch_hook=None):
176 """Make a `BranchFileSystemClient`.
177
178 The created client interacts with the InMemoryFrontend.
179 """
180 return BranchFileSystemClient(
181 self._xmlrpc_client, self.user.id, expiry_time=expiry_time,
182+ seen_new_branch_hook=seen_new_branch_hook,
183 _now=self.fake_time.now)
184
185 def test_translatePath(self):
186@@ -213,6 +214,41 @@
187 return deferred.addCallbacks(
188 translated_successfully, failed_translation)
189
190+ def test_seen_new_branch_hook_called_for_new_branch(self):
191+ # A callable passed as the seen_new_branch_hook when constructing a
192+ # BranchFileSystemClient will be called with a previously unseen
193+ # branch's unique_name when a path for a that branch is translated for
194+ # the first time.
195+ seen_branches = []
196+ client = self.makeClient(seen_new_branch_hook=seen_branches.append)
197+ branch = self.factory.makeAnyBranch()
198+ client.translatePath('/' + branch.unique_name + '/trailing')
199+ self.assertEqual([branch.unique_name], seen_branches)
200+
201+ def test_seen_new_branch_hook_called_for_each_branch(self):
202+ # The seen_new_branch_hook is called for a each branch that is
203+ # accessed.
204+ seen_branches = []
205+ client = self.makeClient(seen_new_branch_hook=seen_branches.append)
206+ branch1 = self.factory.makeAnyBranch()
207+ branch2 = self.factory.makeAnyBranch()
208+ client.translatePath('/' + branch1.unique_name + '/trailing')
209+ client.translatePath('/' + branch2.unique_name + '/trailing')
210+ self.assertEqual(
211+ [branch1.unique_name, branch2.unique_name], seen_branches)
212+
213+ def test_seen_new_branch_hook_called_once_for_a_new_branch(self):
214+ # The seen_new_branch_hook is only called once for a given branch.
215+ seen_branches = []
216+ client = self.makeClient(seen_new_branch_hook=seen_branches.append)
217+ branch1 = self.factory.makeAnyBranch()
218+ branch2 = self.factory.makeAnyBranch()
219+ client.translatePath('/' + branch1.unique_name + '/trailing')
220+ client.translatePath('/' + branch2.unique_name + '/trailing')
221+ client.translatePath('/' + branch1.unique_name + '/different')
222+ self.assertEqual(
223+ [branch1.unique_name, branch2.unique_name], seen_branches)
224+
225
226 class TestTrapFault(TestCase):
227 """Tests for `trap_fault`."""
228
229=== added file 'lib/lp/codehosting/vfs/tests/test_hooks.py'
230--- lib/lp/codehosting/vfs/tests/test_hooks.py 1970-01-01 00:00:00 +0000
231+++ lib/lp/codehosting/vfs/tests/test_hooks.py 2010-03-24 00:47:00 +0000
232@@ -0,0 +1,63 @@
233+# Copyright 2010 Canonical Ltd. This software is licensed under the
234+# GNU Affero General Public License version 3 (see the file LICENSE).
235+
236+"""Tests for the hooks in lp.codehosting.vfs.hooks."""
237+
238+__metaclass__ = type
239+
240+import unittest
241+
242+from lp.codehosting.vfs.hooks import SetProcTitleHook
243+from lp.testing import TestCase
244+
245+
246+class FakeSetProcTitleModule:
247+ """A fake for the setproctitle module.
248+
249+ The `setproctitle` module (obviously) has global effects, so can't really
250+ be used in unit tests. Instances of this class can be used as a safe
251+ replacement.
252+ """
253+
254+ def __init__(self, initial_title):
255+ self.title = initial_title
256+
257+ def getproctitle(self):
258+ return self.title
259+
260+ def setproctitle(self, new_title):
261+ self.title = new_title
262+
263+
264+class TestSetProcTitleHook(TestCase):
265+ """Tests for `SetProcTitleHook`."""
266+
267+ def test_hook_once(self):
268+ # Calling the hook once records the passed branch identifier in the
269+ # process title.
270+ initial_title = self.factory.getUniqueString()
271+ setproctitle_mod = FakeSetProcTitleModule(initial_title)
272+ branch_url = self.factory.getUniqueString()
273+ hook = SetProcTitleHook(setproctitle_mod)
274+ hook.seen(branch_url)
275+ self.assertEqual(
276+ initial_title + " BRANCH:" + branch_url,
277+ setproctitle_mod.getproctitle())
278+
279+ def test_hook_twice(self):
280+ # Calling the hook twice replaces the first branch identifier in the
281+ # process title.
282+ initial_title = self.factory.getUniqueString()
283+ setproctitle_mod = FakeSetProcTitleModule(initial_title)
284+ branch_url1 = self.factory.getUniqueString()
285+ branch_url2 = self.factory.getUniqueString()
286+ hook = SetProcTitleHook(setproctitle_mod)
287+ hook.seen(branch_url1)
288+ hook.seen(branch_url2)
289+ self.assertEqual(
290+ initial_title + " BRANCH:" + branch_url2,
291+ setproctitle_mod.getproctitle())
292+
293+
294+def test_suite():
295+ return unittest.TestLoader().loadTestsFromName(__name__)
296
297=== modified file 'setup.py'
298--- setup.py 2010-03-19 14:25:54 +0000
299+++ setup.py 2010-03-24 00:47:00 +0000
300@@ -54,12 +54,13 @@
301 'pytz',
302 # This appears to be a broken indirect dependency from zope.security:
303 'RestrictedPython',
304+ 'setproctitle',
305 'setuptools',
306 'sourcecodegen',
307 'storm',
308 'testtools',
309 'transaction',
310- 'Twisted',
311+ 'Twisted',
312 'wadllib',
313 'z3c.pt',
314 'z3c.ptcompat',
315
316=== modified file 'versions.cfg'
317--- versions.cfg 2010-03-23 18:27:27 +0000
318+++ versions.cfg 2010-03-24 00:47:00 +0000
319@@ -52,6 +52,7 @@
320 pytz = 2009l
321 RestrictedPython = 3.5.1
322 roman = 1.4.0
323+setproctitle = 1.0
324 setuptools = 0.6c9
325 simplejson = 2.0.9
326 simplesettings = 0.4