Implement package removal via piw-remove

docs/remove.rst

@@ -2,14 +2,14 @@
 piw-remove
 ==========
 
-The piw-remove script is used to manually remove a version of a package from
-the system. All builds for the specified version will be forgotten, and all
-files generated by such builds will be deleted.
+The piw-remove script is used to manually remove a package (or a version of a
+package) from the system. All builds for the specified package (or version)
+will be forgotten, and all files generated by such builds will be deleted.
 
-By default, the version removed will be deleted entirely (*not* marked to
-skip). Optionally, you can provide a skip reason; in this case the version will
-not be deleted (though its builds and files will), but will be left marked to
-skip to prevent future rebuilds.
+By default, the package (or version) removed will be deleted entirely (*not*
+marked to skip). Optionally, you can provide a skip reason; in this case the
+version will not be deleted (though its builds and files will), but will be
+left marked to skip to prevent future rebuilds.
 
 Synopsis
 ========
@@ -18,7 +18,7 @@ Synopsis
 
     piw-remove [-h] [--version] [-c FILE] [-q] [-v] [-l FILE] [-y]
                [-s REASON] [--import-queue ADDR]
-               package version
+               package [version]
 
 
 Description
@@ -32,7 +32,8 @@ Description
 
 .. option:: version
 
-    The version of the package to remove
+    The version of the package to remove. If omitted, removes the entire
+    package
 
 .. option:: -h, --help
 
@@ -90,9 +91,9 @@ The utility can be run in a batch mode with :option:`--yes` but still requires
 invoking once per deletion required (you cannot remove multiple versions in a
 single invocation).
 
-The return code will be 0 if the version was successfully removed. If anything
-fails, the return code will be non-zero and no files should be deleted (but
-this cannot be guaranteed in all circumstances).
+The return code will be 0 if the package (or version) was successfully removed.
+If anything fails, the return code will be non-zero and no files should be
+deleted (but this cannot be guaranteed in all circumstances).
 
 The utility should only ever be run directly on the master node (opening the
 import queue to other machines is a potential security risk).

piwheels/master/__init__.py

@@ -232,8 +232,8 @@ def __call__(self, args=None):
                 TheSecretary,
                 BigBrother,
                 FileJuggler,
-                MrChase,
                 SlaveDriver,
+                MrChase,
                 TheArchitect,
                 CloudGazer,
             )

piwheels/master/mr_chase.py

@@ -69,6 +69,9 @@ def __init__(self, config):
         self.stats_queue = self.socket(
             transport.PUSH, protocol=reversed(protocols.big_brother))
         self.stats_queue.connect(config.stats_queue)
+        self.skip_queue = self.socket(
+            transport.REQ, protocol=protocols.cloud_gazer)
+        self.skip_queue.connect(const.SKIP_QUEUE)
         self.db = DbClient(config, self.logger)
         self.fs = FsClient(config, self.logger)
         self.states = {}
@@ -202,18 +205,44 @@ def do_remove(self, state):
         remove a specific version of a package from the system.
         """
         package, version, reason = state
+        if version is None:
+            return self.do_remove_package(package, reason)
+        else:
+            return self.do_remove_version(package, version, reason)
+
+    def do_remove_package(self, package, reason):
+        if not self.db.test_package(package):
+            self.logger.error('unknown package %s', package)
+            return 'ERROR', 'unknown package %s' % package
+        self.logger.info('removing %s', package)
+        if reason:
+            self.db.skip_package(package, reason)
+        self.web_queue.send_msg('DELPKG', package)
+        self.skip_queue.send_msg('DELPKG', package)
+        self.web_queue.recv_msg()
+        self.skip_queue.recv_msg()
+        if reason:
+            for row in self.db.get_project_versions(package):
+                if row.builds_succeeded:
+                    self.db.delete_build(package, row.version)
+        else:
+            # FKs will take care of removing builds here
+            self.db.delete_package(package)
+        return 'DONE', protocols.NoData
+
+    def do_remove_version(self, package, version, reason):
         if not self.db.test_package_version(package, version):
             self.logger.error('unknown package version %s-%s',
                               package, version)
             return 'ERROR', 'unknown package version %s-%s' % (
                 package, version)
-        self.logger.info('removing %s %s', package, version)
+        self.logger.info('removing %s-%s', package, version)
         if reason:
             self.db.skip_package_version(package, version, reason)
         self.web_queue.send_msg('DELVER', [package, version])
+        self.skip_queue.send_msg('DELVER', [package, version])
         self.web_queue.recv_msg()
-        # XXX Technically we ought to send DELVER to slave-driver's skip-queue
-        # here but let's not complicate the spider's web of connections!
+        self.skip_queue.recv_msg()
         if reason:
             self.db.delete_build(package, version)
         else:

piwheels/protocols.py

@@ -324,7 +324,7 @@ def __reversed__(self):
 
 mr_chase = Protocol(recv={
     'IMPORT': _build_state,
-    'REMOVE': ExactSequence([str, str, str]),  # package, version, skip-reason
+    'REMOVE': ExactSequence([str, Any(str, None), str]),  # package, [version], skip-reason
     'REBUILD': Any(
         ExactSequence(['HOME']),
         ExactSequence(['SEARCH']),

piwheels/remove/__init__.py

@@ -58,13 +58,17 @@ def main(args=None):
     parser.add_argument(
         'package', default=None, help="The name of the package to remove")
     parser.add_argument(
-        'version', default=None, help="The version of the package to remove")
+        'version', nargs='?', default=None,
+        help="The version of the package to remove; if omitted, removes the "
+        "entire package")
     parser.add_argument(
         '-y', '--yes', action='store_true',
         help="Run non-interactively; never prompt during operation")
     parser.add_argument(
         '-s', '--skip', action='store', default='', metavar='REASON',
-        help="Mark the version with a reason to prevent future build attempts")
+        help="Don't delete the package / version from the database, but mark "
+        "it with a reason to prevent future build attempts (wheels will still "
+        "be removed)")
     parser.add_argument(
         '--import-queue', metavar='ADDR', default=const.IMPORT_QUEUE,
         help="The address of the queue used by piw-remove (default: "
@@ -75,8 +79,13 @@ def main(args=None):
     logging.info("PiWheels Remover version %s", __version__)
 
     if not config.yes:
-        logging.warning("Preparing to remove all wheels for %s %s",
-                        config.package, config.version)
+        action = 'remove all wheels' if config.skip else 'delete'
+        if config.version is not None:
+            logging.warning("Preparing to %s for %s %s",
+                            action, config.package, config.version)
+        else:
+            logging.warning("Preparing to %s for %s",
+                            action, config.package)
         if not terminal.yes_no_prompt('Proceed?'):
             logging.warning('User aborted removal')
             return 2

tests/conftest.py

@@ -688,3 +688,11 @@ def web_queue(request, zmq_context, master_config):
                     protocols.the_scribe)
     yield task
     task.close()
+
+
+@pytest.fixture()
+def skip_queue(request, zmq_context, master_config):
+    task = MockTask(zmq_context, transport.REP, const.SKIP_QUEUE,
+                    reversed(protocols.cloud_gazer))
+    yield task
+    task.close()

tests/master/test_cloud_gazer.py

@@ -58,14 +58,6 @@ def events_iter():
         yield source
 
 
-@pytest.fixture()
-def skip_queue(request, zmq_context, master_config):
-    task = MockTask(zmq_context, transport.REP, const.SKIP_QUEUE,
-                    reversed(protocols.cloud_gazer))
-    yield task
-    task.close()
-
-
 @pytest.fixture(scope='function')
 def task(request, db_queue, web_queue, skip_queue, master_config):
     task = CloudGazer(master_config)

tests/master/test_mr_chase.py

@@ -33,6 +33,7 @@
 from piwheels import protocols, transport
 from piwheels.master.mr_chase import MrChase
 from piwheels.master.slave_driver import build_armv6l_hack
+from piwheels.master.the_oracle import ProjectVersionsRow
 
 
 @pytest.fixture()
@@ -321,25 +322,75 @@ def test_import_transfer_goes_wrong(db_queue, fs_queue, task, import_queue,
     assert task.logger.error.call_count == 1
 
 
-def test_normal_remove(db_queue, web_queue, task, import_queue,
-                       build_state_hacked):
+def test_remove_package(db_queue, web_queue, skip_queue, task, import_queue,
+                        build_state_hacked):
+    bsh = build_state_hacked
+    import_queue.send_msg('REMOVE', [bsh.package, None, ''])
+    db_queue.expect('PKGEXISTS', bsh.package)
+    db_queue.send('OK', True)
+    web_queue.expect('DELPKG', bsh.package)
+    web_queue.send('DONE')
+    skip_queue.expect('DELPKG', bsh.package)
+    skip_queue.send('OK')
+    db_queue.expect('DELPKG', bsh.package)
+    db_queue.send('OK', None)
+    task.poll(0)
+    assert import_queue.recv_msg() == ('DONE', None)
+    assert len(task.states) == 0
+    db_queue.check()
+    web_queue.check()
+    skip_queue.check()
+
+
+def test_remove_version(db_queue, web_queue, skip_queue, task, import_queue,
+                        build_state_hacked):
     bsh = build_state_hacked
     import_queue.send_msg('REMOVE', [bsh.package, bsh.version, ''])
     db_queue.expect('VEREXISTS', [bsh.package, bsh.version])
     db_queue.send('OK', True)
     web_queue.expect('DELVER', [bsh.package, bsh.version])
     web_queue.send('DONE')
+    skip_queue.expect('DELVER', [bsh.package, bsh.version])
+    skip_queue.send('OK')
     db_queue.expect('DELVER', [bsh.package, bsh.version])
     db_queue.send('OK', None)
     task.poll(0)
     assert import_queue.recv_msg() == ('DONE', None)
     assert len(task.states) == 0
     db_queue.check()
     web_queue.check()
+    skip_queue.check()
 
 
-def test_remove_with_skip(db_queue, web_queue, task, import_queue,
-                          build_state_hacked):
+def test_skip_package(db_queue, web_queue, skip_queue, task, import_queue,
+                      build_state_hacked):
+    bsh = build_state_hacked
+    import_queue.send_msg('REMOVE', [bsh.package, None, 'silly package'])
+    db_queue.expect('PKGEXISTS', bsh.package)
+    db_queue.send('OK', True)
+    db_queue.expect('SKIPPKG', [bsh.package, 'silly package'])
+    db_queue.send('OK', None)
+    web_queue.expect('DELPKG', bsh.package)
+    web_queue.send('DONE')
+    skip_queue.expect('DELPKG', bsh.package)
+    skip_queue.send('OK')
+    db_queue.expect('PROJVERS', bsh.package)
+    db_queue.send('OK', [
+        ProjectVersionsRow(bsh.version, False, 'cp34m', '', False),
+        ProjectVersionsRow(bsh.version + 'a', False, '', 'cp35m', False),
+    ])
+    db_queue.expect('DELBUILD', [bsh.package, bsh.version])
+    db_queue.send('OK', None)
+    task.poll(0)
+    assert import_queue.recv_msg() == ('DONE', None)
+    assert len(task.states) == 0
+    db_queue.check()
+    web_queue.check()
+    skip_queue.check()
+
+
+def test_skip_version(db_queue, web_queue, skip_queue, task, import_queue,
+                      build_state_hacked):
     bsh = build_state_hacked
     import_queue.send_msg('REMOVE', [bsh.package, bsh.version, 'broken version'])
     db_queue.expect('VEREXISTS', [bsh.package, bsh.version])
@@ -348,20 +399,38 @@ def test_remove_with_skip(db_queue, web_queue, task, import_queue,
     db_queue.send('OK', None)
     web_queue.expect('DELVER', [bsh.package, bsh.version])
     web_queue.send('DONE')
+    skip_queue.expect('DELVER', [bsh.package, bsh.version])
+    skip_queue.send('OK')
     db_queue.expect('DELBUILD', [bsh.package, bsh.version])
     db_queue.send('OK', None)
     task.poll(0)
     assert import_queue.recv_msg() == ('DONE', None)
     assert len(task.states) == 0
     db_queue.check()
     web_queue.check()
+    skip_queue.check()
 
 
 def test_remove_unknown_pkg(db_queue, task, import_queue, build_state):
     task.logger = mock.Mock()
     build_state._slave_id = 0
     bs = build_state
 
+    import_queue.send_msg('REMOVE', [bs.package, None, ''])
+    db_queue.expect('PKGEXISTS', bs.package)
+    db_queue.send('OK', False)
+    task.poll(0)
+    assert import_queue.recv_msg() == (
+        'ERROR', 'unknown package %s' % bs.package)
+    assert task.logger.error.call_count == 1
+    assert len(task.states) == 0
+
+
+def test_remove_unknown_ver(db_queue, task, import_queue, build_state):
+    task.logger = mock.Mock()
+    build_state._slave_id = 0
+    bs = build_state
+
     import_queue.send_msg('REMOVE', [bs.package, bs.version, ''])
     db_queue.expect('VEREXISTS', [bs.package, bs.version])
     db_queue.send('OK', False)

tests/remove/test_remove.py

@@ -96,7 +96,17 @@ def test_abort(caplog):
     assert find_message(caplog.records, message='User aborted removal')
 
 
-def test_remove(mock_context, import_queue_name, import_queue):
+def test_remove_package(mock_context, import_queue_name, import_queue):
+    with mock.patch('piwheels.terminal.yes_no_prompt') as prompt_mock:
+        prompt_mock.return_value = True
+        with RemoveThread(['--import-queue', import_queue_name, 'foo']) as thread:
+            assert import_queue.recv_msg() == ('REMOVE', ['foo', None, ''])
+            import_queue.send_msg('DONE')
+            thread.join(10)
+            assert thread.exitcode == 0
+
+
+def test_remove_version(mock_context, import_queue_name, import_queue):
     with mock.patch('piwheels.terminal.yes_no_prompt') as prompt_mock:
         prompt_mock.return_value = True
         with RemoveThread(['--import-queue', import_queue_name, 'foo', '0.1']) as thread: