Merge lp:~andrewjbeach/juju-release-tools/s3-container-arg into lp:juju-release-tools

[Andrew James Beach]

Added -s/--s3-container as a command line argument to agent_archive.py.

The preset value is now only used as a default, the value is passed down to
where it is used.

The test file now uses a fake value to help sperate the tests from any actual
runs of the script.

In addition I fixed some things scattered throughout that was causing
`make lint` to complain, just to clean the board.

Some more tests should be added but the diff was getting rather large.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== modified file 'agent_archive.py'
--- agent_archive.py	2016-02-17 15:48:39 +0000
+++ agent_archive.py	2016-11-29 22:29:55 +0000
@@ -12,7 +12,7 @@
 
 
 # The S3 container and path to add to and get from.
-S3_CONTAINER = 's3://juju-qa-data/agent-archive'
+S3_CONTAINER_DEFAULT = 's3://juju-qa-data/agent-archive'
 # The set of agents to make.
 AGENT_TEMPLATES = (
     'juju-{}-centos7-amd64.tgz',
@@ -26,7 +26,7 @@
     'juju-{}-win8-amd64.tgz',
     'juju-{}-win81-amd64.tgz',
     'juju-{}-win10-amd64.tgz',
-)
+    )
 # The versions of agent that may or will exist. The agents will
 # always start with juju, the series will start with "win" and the
 # arch is always amd64.
@@ -70,7 +70,7 @@
 
     Wrap deprecated raw_input for testing.
     """
-    return raw_input(prompt)  # pyflakes:ignore
+    return raw_input(prompt)  # NOQA
 
 
 def listing_to_files(listing):
@@ -82,7 +82,7 @@
     return agents
 
 
-def is_new_version(source_path, config, verbose=False):
+def is_new_version(source_path, config, s3_container, verbose=False):
     """Return True when the version is new, else False.
 
     :raises: ValueError if the version exists and is different.
@@ -91,7 +91,7 @@
     if verbose:
         print('Checking that %s does not already exist.' % source_path)
     source_agent = os.path.basename(source_path)
-    agent_path = '%s/%s' % (S3_CONTAINER, source_agent)
+    agent_path = '%s/%s' % (s3_container, source_agent)
     existing_version = run(
         ['ls', '--list-md5', agent_path], config=config, verbose=verbose)
     if not existing_version:
@@ -128,22 +128,23 @@
         raise ValueError(
             '%s does not match an expected version.' % source_agent)
     source_path = os.path.abspath(os.path.expanduser(args.source_agent))
-    if not is_new_version(source_path, args.config, verbose=args.verbose):
+    if not is_new_version(source_path, args.config, args.s3_container,
+                          verbose=args.verbose):
         if args.verbose:
             print("Nothing to do.")
         return
     # The fastest way to put the files in place is to upload the source_agent
     # then use the s3cmd cp to make remote versions.
     if args.verbose:
-        print('Uploading %s to %s' % (source_agent, S3_CONTAINER))
-    remote_source = '%s/%s' % (S3_CONTAINER, source_agent)
+        print('Uploading %s to %s' % (source_agent, args.s3_container))
+    remote_source = '%s/%s' % (args.s3_container, source_agent)
     run(['put', source_path, remote_source],
         config=args.config, dry_run=args.dry_run, verbose=args.verbose)
     agent_versions.remove(source_agent)
     os_name = get_source_agent_os(source_agent)
     agent_versions = [a for a in agent_versions if os_name in a]
     for agent_version in agent_versions:
-        destination = '%s/%s' % (S3_CONTAINER, agent_version)
+        destination = '%s/%s' % (args.s3_container, agent_version)
         if args.verbose:
             print('Copying %s to %s' % (remote_source, destination))
         run(['cp', remote_source, destination],
@@ -153,7 +154,7 @@
 def get_agents(args):
     """Download agents matching a version to a destination path."""
     version = args.version
-    agent_glob = '%s/juju-%s*' % (S3_CONTAINER, version)
+    agent_glob = '%s/juju-%s*' % (args.s3_container, version)
     destination = os.path.abspath(os.path.expanduser(args.destination))
     output = run(
         ['get', agent_glob, destination],
@@ -169,7 +170,7 @@
     matches the expected operation.
     """
     version = args.version
-    agent_glob = '%s/juju-%s*' % (S3_CONTAINER, version)
+    agent_glob = '%s/juju-%s*' % (args.s3_container, version)
     existing_versions = run(
         ['ls', agent_glob], config=args.config, verbose=args.verbose)
     if args.verbose:
@@ -201,6 +202,9 @@
     parser.add_argument(
         '-c', '--config', action='store', default=None,
         help='The S3 config file.')
+    parser.add_argument(
+        '-s', '--s3-container', action='store', default=S3_CONTAINER_DEFAULT,
+        help='The S3 container to act on.')
     subparsers = parser.add_subparsers(help='sub-command help')
     # add juju-1.21.0-win2012-amd64.tgz
     parser_add = subparsers.add_parser(

=== modified file 'lprelease.py'
--- lprelease.py	2016-09-29 21:51:22 +0000
+++ lprelease.py	2016-11-29 22:29:55 +0000
@@ -97,10 +97,11 @@
 def defer_bugs(milestone, deferred_milestone, dry_run, verbose):
     unfinished_bug_tasks = milestone.searchTasks(status=UNFINISHED)
     for bug_task in unfinished_bug_tasks:
-		#This needs fixed; bug #1629115
-        #if verbose:
-        #    print('Retargeting bug %s [%s] to %s' % (
-        #        bug_task.bug.id, bug_task.bug.title, deferred_milestone.name))
+        # This needs fixed; bug #1629115
+        # if verbose:
+        #     print('Retargeting bug %s [%s] to %s' % (
+        #         bug_task.bug.id, bug_task.bug.title,
+        #         deferred_milestone.name))
         bug_task.milestone = deferred_milestone
         if not dry_run:
             bug_task.lp_save()

=== modified file 'tests/test_agent_archive.py'
--- tests/test_agent_archive.py	2016-02-17 15:48:39 +0000
+++ tests/test_agent_archive.py	2016-11-29 22:29:55 +0000
@@ -16,19 +16,25 @@
 from utils import temp_dir
 
 
+# Name of a non-existant s3-container.
+S3_CONTAINER_FAKE = 's3://juju-qa-fake/agent-archive'
+
+
 class FakeArgs:
 
     def __init__(self, source_agent=None, version=None, destination=None,
-                 config=None, verbose=False, dry_run=False):
+                 config=None, verbose=False, dry_run=False,
+                 s3_container=S3_CONTAINER_FAKE):
         self.source_agent = source_agent
         self.version = version
         self.destination = destination
+        self.s3_container = s3_container
         self.config = None
         self.verbose = verbose
         self.dry_run = dry_run
 
 
-class AgentArchive(TestCase):
+class TestAgentArchive(TestCase):
 
     def test_main_options(self):
         with patch('agent_archive.add_agents') as mock:
@@ -108,7 +114,7 @@
             get_source_agent_os('juju-1.24.footu-amd64.tgz')
 
     def test_listing_to_files(self):
-        start = '2014-10-23 22:11  9820182  s3://juju-qa-data/agent-archive/%s'
+        start = '2014-10-23 22:11  9820182  s3://juju-qa-fake/agent-archive/%s'
         listing = []
         expected_agents = []
         agents = [
@@ -118,15 +124,16 @@
         for agent in agents:
             listing.append(start % agent)
             expected_agents.append(
-                's3://juju-qa-data/agent-archive/%s' % agent)
+                's3://juju-qa-fake/agent-archive/%s' % agent)
         agents = listing_to_files('\n'.join(listing))
         self.assertEqual(expected_agents, agents)
 
     def test_is_new_version(self):
-        agent = 's3://juju-qa-data/agent-archive/juju-1.21.0-win2012-amd64.tgz'
+        agent = 's3://juju-qa-fake/agent-archive/juju-1.21.0-win2012-amd64.tgz'
         with patch('agent_archive.run', return_value='') as mock:
             result = is_new_version(
-                'juju-1.21.0-win2012-amd64.tgz', 'config', verbose=False)
+                'juju-1.21.0-win2012-amd64.tgz', 'config',
+                S3_CONTAINER_FAKE, verbose=False)
         self.assertTrue(result)
         mock.assert_called_with(
             ['ls', '--list-md5', agent], config='config', verbose=False)
@@ -134,26 +141,27 @@
     def test_is_new_version_idential(self):
         listing = (
             '2015-05-27 14:16   8292541   b33aed8f3134996703dc39f9a7c95783  '
-            's3://juju-qa-data/agent-archive/juju-1.21.0-win2012-amd64.tgz')
+            's3://juju-qa-fake/agent-archive/juju-1.21.0-win2012-amd64.tgz')
         with temp_dir() as base:
             local_agent = os.path.join(base, 'juju-1.21.0-win2012-amd64.tgz')
             with open(local_agent, 'w') as f:
                 f.write('agent')
             with patch('agent_archive.run', return_value=listing):
-                result = is_new_version(local_agent, 'config')
+                result = is_new_version(local_agent, 'config',
+                                        S3_CONTAINER_FAKE)
         self.assertFalse(result)
 
     def test_is_new_version_not_identical_error(self):
         listing = (
             '2015-05-27 14:16   8292541   69988f8072c3839fa2a364d80a652f3f  '
-            's3://juju-qa-data/agent-archive/juju-1.21.0-win2012-amd64.tgz')
+            's3://juju-qa-fake/agent-archive/juju-1.21.0-win2012-amd64.tgz')
         with temp_dir() as base:
             local_agent = os.path.join(base, 'juju-1.21.0-win2012-amd64.tgz')
             with open(local_agent, 'w') as f:
                 f.write('agent')
             with patch('agent_archive.run', return_value=listing):
                 with self.assertRaises(ValueError) as e:
-                    is_new_version(local_agent, 'config')
+                    is_new_version(local_agent, 'config', S3_CONTAINER_FAKE)
         self.assertIn('Agents cannot be changed', str(e.exception))
 
     def test_add_agent_with_bad_source_raises_error(self):
@@ -179,7 +187,8 @@
             with self.assertRaises(ValueError):
                 add_agents(cmd_args)
         agent_path = os.path.abspath(cmd_args.source_agent)
-        nv_mock.assert_called_with(agent_path, None, verbose=False)
+        nv_mock.assert_called_with(agent_path, None, S3_CONTAINER_FAKE,
+                                   verbose=False)
 
     def test_add_agent_puts_and_copies_win(self):
         cmd_args = FakeArgs(source_agent='juju-1.21.0-win2012-amd64.tgz')
@@ -189,27 +198,27 @@
                 add_agents(cmd_args)
         nv_mock.assert_called_with(
             os.path.abspath('juju-1.21.0-win2012-amd64.tgz'),
-            None, verbose=False)
+            None, S3_CONTAINER_FAKE, verbose=False)
         self.assertEqual(10, mock.call_count)
         output, args, kwargs = mock.mock_calls[0]
         agent_path = os.path.abspath(cmd_args.source_agent)
         self.assertEqual(
             ['put', agent_path,
-             's3://juju-qa-data/agent-archive/juju-1.21.0-win2012-amd64.tgz'],
+             's3://juju-qa-fake/agent-archive/juju-1.21.0-win2012-amd64.tgz'],
             args[0])
         # The remaining calls after the put is a fast cp to the other names.
         output, args, kwargs = mock.mock_calls[1]
         self.assertEqual(
             ['cp',
-             's3://juju-qa-data/agent-archive/juju-1.21.0-win2012-amd64.tgz',
-             's3://juju-qa-data/agent-archive/'
+             's3://juju-qa-fake/agent-archive/juju-1.21.0-win2012-amd64.tgz',
+             's3://juju-qa-fake/agent-archive/'
              'juju-1.21.0-win2012hvr2-amd64.tgz'],
             args[0])
         output, args, kwargs = mock.mock_calls[8]
         self.assertEqual(
             ['cp',
-             's3://juju-qa-data/agent-archive/juju-1.21.0-win2012-amd64.tgz',
-             's3://juju-qa-data/agent-archive/juju-1.21.0-win81-amd64.tgz'],
+             's3://juju-qa-fake/agent-archive/juju-1.21.0-win2012-amd64.tgz',
+             's3://juju-qa-fake/agent-archive/juju-1.21.0-win81-amd64.tgz'],
             args[0])
 
     def test_add_agent_puts_centos(self):
@@ -219,12 +228,13 @@
                        return_value=True) as nv_mock:
                 add_agents(cmd_args)
         agent_path = os.path.abspath(cmd_args.source_agent)
-        nv_mock.assert_called_with(agent_path, None, verbose=False)
+        nv_mock.assert_called_with(agent_path, None, S3_CONTAINER_FAKE,
+                                   verbose=False)
         self.assertEqual(1, mock.call_count)
         agent_path = os.path.abspath(cmd_args.source_agent)
         mock.assert_called_with(
             ['put', agent_path,
-             's3://juju-qa-data/agent-archive/juju-1.24.0-centos7-amd64.tgz'],
+             's3://juju-qa-fake/agent-archive/juju-1.24.0-centos7-amd64.tgz'],
             config=None, verbose=False, dry_run=False)
 
     def test_get_agent(self):
@@ -234,7 +244,7 @@
             get_agents(cmd_args)
         args, kwargs = mock.call_args
         self.assertEqual(
-            (['get', 's3://juju-qa-data/agent-archive/juju-1.21.0*',
+            (['get', 's3://juju-qa-fake/agent-archive/juju-1.21.0*',
               destination], ),
             args)
 
@@ -246,7 +256,7 @@
         self.assertIn('No 1.21.0 agents found', str(e.exception))
         args, kwargs = mock.call_args
         self.assertEqual(
-            (['ls', 's3://juju-qa-data/agent-archive/juju-1.21.0*'], ),
+            (['ls', 's3://juju-qa-fake/agent-archive/juju-1.21.0*'], ),
             args)
         self.assertIs(None, kwargs['config'], )
 
@@ -259,12 +269,12 @@
         self.assertEqual(1, mock.call_count)
         args, kwargs = mock.call_args
         self.assertEqual(
-            (['ls', 's3://juju-qa-data/agent-archive/juju-1.21.0*'], ),
+            (['ls', 's3://juju-qa-fake/agent-archive/juju-1.21.0*'], ),
             args)
 
     def test_delete_agent_with_yes(self):
         cmd_args = FakeArgs(version='1.21.0')
-        start = '2014-10-23 22:11  9820182  s3://juju-qa-data/agent-archive/%s'
+        start = '2014-10-23 22:11  9820182  s3://juju-qa-fake/agent-archive/%s'
         listing = []
         agents = [
             'juju-1.21.0-win2012-amd64.tgz',
@@ -279,17 +289,17 @@
         self.assertEqual(3, mock.call_count)
         output, args, kwargs = mock.mock_calls[0]
         self.assertEqual(
-            ['ls', 's3://juju-qa-data/agent-archive/juju-1.21.0*'],
+            ['ls', 's3://juju-qa-fake/agent-archive/juju-1.21.0*'],
             args[0])
         output, args, kwargs = mock.mock_calls[1]
         self.assertEqual(
             ['del',
-             's3://juju-qa-data/agent-archive/juju-1.21.0-win2012-amd64.tgz'],
+             's3://juju-qa-fake/agent-archive/juju-1.21.0-win2012-amd64.tgz'],
             args[0])
         output, args, kwargs = mock.mock_calls[2]
         self.assertEqual(
             ['del',
-             's3://juju-qa-data/agent-archive/juju-1.21.0-win8.1-amd64.tgz'],
+             's3://juju-qa-fake/agent-archive/juju-1.21.0-win8.1-amd64.tgz'],
             args[0])
         self.assertIs(None, kwargs['config'])
         self.assertFalse(kwargs['dry_run'])

=== modified file 'tests/test_make_release_notes.py'
--- tests/test_make_release_notes.py	2016-08-25 19:00:50 +0000
+++ tests/test_make_release_notes.py	2016-11-29 22:29:55 +0000
@@ -123,7 +123,7 @@
         self.assertIn(
             'https://jujucharms.com/docs/devel/temp-release-notes', text)
         self.assertIn('snap install juju --beta --devmode', text)
-                
+
     def test_make_notes_with_notable(self):
         # The default value of None implies a stable bug fix release.
         text = make_notes('1.20.1', DEVEL, "* One\n  Lp 1", notable=None)