launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17721
[Merge] lp:~cjwatson/launchpad/remove-upload-locking into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-upload-locking into lp:launchpad.
Commit message:
Remove package upload locking and obsolete .distro files, now that txpkgupload moves uploads into place atomically.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-upload-locking/+merge/247461
Remove package upload locking and obsolete .distro files, now that txpkgupload moves uploads into place atomically.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-upload-locking into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/livefsupload.py'
--- lib/lp/archiveuploader/livefsupload.py 2014-06-16 09:44:24 +0000
+++ lib/lp/archiveuploader/livefsupload.py 2015-01-23 17:58:57 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014 Canonical Ltd. This software is licensed under the
+# Copyright 2014-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Process a live filesystem upload."""
@@ -38,9 +38,7 @@
for dirpath, _, filenames in os.walk(self.upload_path):
if dirpath == self.upload_path:
- # All relevant files will be in a subdirectory; this is a
- # simple way to avoid uploading any .distro file that may
- # exist.
+ # All relevant files will be in a subdirectory.
continue
for livefs_file in sorted(filenames):
livefs_path = os.path.join(dirpath, livefs_file)
=== modified file 'lib/lp/archiveuploader/tests/test_processupload.py'
--- lib/lp/archiveuploader/tests/test_processupload.py 2014-08-09 19:45:00 +0000
+++ lib/lp/archiveuploader/tests/test_processupload.py 2015-01-23 17:58:57 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -75,29 +75,3 @@
# Explicitly mark the database dirty.
self.layer.force_dirty_database()
-
- def testTopLevelLockFile(self):
- """Try a simple process-upload run.
-
- Expect it to exit earlier due the occupied lockfile
- """
- # acquire the process-upload lockfile locally
- from contrib.glock import GlobalLock
- locker = GlobalLock('/var/lock/process-upload-insecure.lock')
- locker.acquire()
-
- returncode, out, err = self.runProcessUpload(
- extra_args=['-C', 'insecure'])
-
- # the process-upload call terminated with ERROR and
- # proper log message
- self.assertEqual(1, returncode)
- self.assert_(
- 'INFO Creating lockfile: '
- '/var/lock/process-upload-insecure.lock' in err.splitlines())
- self.assert_(
- 'INFO Lockfile /var/lock/process-upload-insecure.lock in use'
- in err.splitlines())
-
- # release the locally acquired lockfile
- locker.release()
=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py 2014-10-31 07:00:29 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py 2015-01-23 17:58:57 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Functional tests for uploadprocessor.py."""
@@ -438,11 +438,7 @@
self.getUploadProcessor(None)
def testLocateDirectories(self):
- """Return a sorted list of subdirs in a directory.
-
- We don't test that we block on the lockfile, as this is trivial
- code but tricky to test.
- """
+ """Return a sorted list of subdirs in a directory."""
testdir = tempfile.mkdtemp()
try:
os.mkdir("%s/dir3" % testdir)
@@ -457,19 +453,14 @@
def _makeUpload(self, testdir):
"""Create a dummy upload for testing the move/remove methods."""
- upload = tempfile.mkdtemp(dir=testdir)
- distro = upload + ".distro"
- f = open(distro, mode="w")
- f.write("foo")
- f.close()
- return upload, distro
+ return tempfile.mkdtemp(dir=testdir)
def testMoveUpload(self):
- """moveUpload should move the upload directory and .distro file."""
+ """moveUpload should move the upload directory."""
testdir = tempfile.mkdtemp()
try:
- # Create an upload, a .distro and a target to move it to.
- upload, distro = self._makeUpload(testdir)
+ # Create an upload and a target to move it to.
+ upload = self._makeUpload(testdir)
target = tempfile.mkdtemp(dir=testdir)
target_name = os.path.basename(target)
upload_name = os.path.basename(upload)
@@ -482,19 +473,15 @@
# Check it moved
self.assertTrue(os.path.exists(os.path.join(target, upload_name)))
- self.assertTrue(os.path.exists(os.path.join(
- target, upload_name + ".distro")))
self.assertFalse(os.path.exists(upload))
- self.assertFalse(os.path.exists(distro))
finally:
shutil.rmtree(testdir)
def testMoveProcessUploadAccepted(self):
testdir = tempfile.mkdtemp()
try:
- # Create an upload, a .distro and a target to move it to.
- upload, distro = self._makeUpload(testdir)
- upload_name = os.path.basename(upload)
+ # Create an upload and a target to move it to.
+ upload = self._makeUpload(testdir)
# Remove it
self.options.base_fsroot = testdir
@@ -503,12 +490,8 @@
handler.moveProcessedUpload("accepted", self.log)
# Check it was removed, not moved
- self.assertFalse(os.path.exists(os.path.join(
- testdir, "accepted")))
- self.assertFalse(os.path.exists(os.path.join(testdir,
- "accepted", upload_name + ".distro")))
+ self.assertFalse(os.path.exists(os.path.join(testdir, "accepted")))
self.assertFalse(os.path.exists(upload))
- self.assertFalse(os.path.exists(distro))
finally:
shutil.rmtree(testdir)
@@ -516,8 +499,8 @@
"""moveProcessedUpload moves if the result was not successful."""
testdir = tempfile.mkdtemp()
try:
- # Create an upload, a .distro and a target to move it to.
- upload, distro = self._makeUpload(testdir)
+ # Create an upload and a target to move it to.
+ upload = self._makeUpload(testdir)
upload_name = os.path.basename(upload)
# Move it
@@ -527,21 +510,18 @@
handler.moveProcessedUpload("rejected", self.log)
# Check it moved
- self.assertTrue(os.path.exists(os.path.join(testdir,
- "rejected", upload_name)))
- self.assertTrue(os.path.exists(os.path.join(testdir,
- "rejected", upload_name + ".distro")))
+ self.assertTrue(os.path.exists(os.path.join(
+ testdir, "rejected", upload_name)))
self.assertFalse(os.path.exists(upload))
- self.assertFalse(os.path.exists(distro))
finally:
shutil.rmtree(testdir)
def testRemoveUpload(self):
- """RemoveUpload removes the upload directory and .distro file."""
+ """RemoveUpload removes the upload directory."""
testdir = tempfile.mkdtemp()
try:
- # Create an upload, a .distro and a target to move it to.
- upload, distro = self._makeUpload(testdir)
+ # Create an upload and a target to move it to.
+ upload = self._makeUpload(testdir)
os.path.basename(upload)
# Remove it
@@ -551,10 +531,8 @@
handler.removeUpload(self.log)
# Check it was removed, not moved
- self.assertFalse(os.path.exists(os.path.join(
- testdir, "accepted")))
+ self.assertFalse(os.path.exists(os.path.join(testdir, "accepted")))
self.assertFalse(os.path.exists(upload))
- self.assertFalse(os.path.exists(distro))
finally:
shutil.rmtree(testdir)
=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py 2014-08-21 03:00:46 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py 2015-01-23 17:58:57 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Code for 'processing' 'uploads'. Also see nascentupload.py.
@@ -49,10 +49,8 @@
import os
import shutil
-import stat
import sys
-from contrib.glock import GlobalLock
from sqlobject import SQLObjectNotFound
from zope.component import getUtility
@@ -204,46 +202,15 @@
def locateDirectories(self, fsroot):
"""Return a list of upload directories in a given queue.
- This method operates on the queue atomically, i.e. it suppresses
- changes in the queue directory, like new uploads, by acquiring
- the shared upload_queue lockfile while the directory are listed.
-
:param fsroot: path to a 'queue' directory to be inspected.
:return: a list of upload directories found in the queue
alphabetically sorted.
"""
- # Protecting listdir by a lock ensures that we only get completely
- # finished directories listed. See lp.poppy.hooks for the other
- # locking place.
- lockfile_path = os.path.join(fsroot, ".lock")
- fsroot_lock = GlobalLock(lockfile_path)
- mode = stat.S_IMODE(os.stat(lockfile_path).st_mode)
-
- # XXX cprov 20081024 bug=185731: The lockfile permission can only be
- # changed by its owner. Since we can't predict which process will
- # create it in production systems we simply ignore errors when trying
- # to grant the right permission. At least, one of the process will
- # be able to do so.
- try:
- os.chmod(lockfile_path, mode | stat.S_IWGRP)
- except OSError as err:
- self.log.debug('Could not fix the lockfile permission: %s' % err)
-
- try:
- fsroot_lock.acquire(blocking=True)
- dir_names = os.listdir(fsroot)
- finally:
- # Skip lockfile deletion, see similar code in lp.poppy.hooks.
- fsroot_lock.release(skip_delete=True)
-
- sorted_dir_names = sorted(
- dir_name
- for dir_name in dir_names
+ return sorted(
+ dir_name for dir_name in os.listdir(fsroot)
if os.path.isdir(os.path.join(fsroot, dir_name)))
- return sorted_dir_names
-
class UploadHandler:
"""Handler for processing a single upload."""
@@ -301,9 +268,8 @@
"""Process a single changes file.
This is done by obtaining the appropriate upload policy (according
- to command-line options and the value in the .distro file beside
- the upload, if present), creating a NascentUpload object and calling
- its process method.
+ to command-line options), creating a NascentUpload object and
+ calling its process method.
We obtain the context for this processing from the relative path,
within the upload folder, of this changes file. This influences
@@ -475,9 +441,6 @@
def removeUpload(self, logger):
"""Remove an upload that has succesfully been processed.
- This includes moving the given upload directory and moving the
- matching .distro file, if it exists.
-
:param logger: The logger to use for logging results.
"""
if self.processor.keep or self.processor.dry_run:
@@ -487,11 +450,6 @@
logger.debug("Removing upload directory %s", self.upload_path)
shutil.rmtree(self.upload_path)
- distro_filename = self.upload_path + ".distro"
- if os.path.isfile(distro_filename):
- logger.debug("Removing distro file %s", distro_filename)
- os.remove(distro_filename)
-
def moveProcessedUpload(self, destination, logger):
"""Move or remove the upload depending on the status of the upload.
@@ -506,9 +464,6 @@
def moveUpload(self, subdir_name, logger):
"""Move the upload to the named subdir of the root, eg 'accepted'.
- This includes moving the given upload directory and moving the
- matching .distro file, if it exists.
-
:param subdir_name: Name of the subdirectory to move to.
:param logger: The logger to use for logging results.
"""
@@ -524,15 +479,6 @@
(self.upload_path, target_path))
shutil.move(self.upload_path, target_path)
- distro_filename = self.upload_path + ".distro"
- if os.path.isfile(distro_filename):
- target_path = os.path.join(self.processor.base_fsroot,
- subdir_name,
- os.path.basename(distro_filename))
- logger.debug("Moving distro file %s to %s" % (distro_filename,
- target_path))
- shutil.move(distro_filename, target_path)
-
@staticmethod
def orderFilenames(fnames):
"""Order filenames, sorting *_source.changes before others.
=== modified file 'versions.cfg'
--- versions.cfg 2015-01-21 11:46:31 +0000
+++ versions.cfg 2015-01-23 17:58:57 +0000
@@ -54,7 +54,7 @@
lazr.restful = 0.19.10
lazr.restfulclient = 0.13.2
lazr.smtptest = 1.3
-lazr.sshserver = 0.1
+lazr.sshserver = 0.1.1
lazr.testing = 0.1.1
lazr.uri = 1.0.2
lpjsmin = 0.5
@@ -127,7 +127,7 @@
txfixtures = 0.1.4
txlongpoll = 0.2.12
txlongpollfixture = 0.1.3
-txpkgupload = 0.1.1
+txpkgupload = 0.2
unittest2 = 0.5.1
van.testing = 3.0.0
wadllib = 1.3.2
Follow ups
