Repository: kangasta/pullnrun

Issue:
Add details logging for S3 pull and push

Discussion:

Patch:
diff --git a/pullnrun/_log.py b/pullnrun/_log.py
index 89198bd..02bf8c3 100644
--- a/pullnrun/_log.py
+++ b/pullnrun/_log.py
@@ -1,18 +1,47 @@
+def _http_details(output_dict):
+    type_ = output_dict.get('type', '')
+    status = str(output_dict.get('data', {}).get('status', '')).rjust(4)
+
+    file_ = output_dict.get('data', {}).get('file', '')
+    direction = 'to' if 'push' in type_ else 'from'
+    url = output_dict.get('data', {}).get('url', '')
+
+    detail = f'{file_} {direction} {url}'
+
+    return (status, detail, )
+
+def _s3_details(output_dict):
+    type_ = output_dict.get('type', '')
+
+    filename = output_dict.get('data', {}).get('filename', '')
+    bucket = output_dict.get('data', {}).get('bucket', '')
+    object_name = output_dict.get('data', {}).get('object_name', '')
+
+    if 'push' in type_:
+        direction = 'to'
+        source, target = filename, object_name
+    else:
+        direction = 'from'
+        source, target = object_name, filename
+
+    target = f' as {target}' if source != target else ''
+    detail = f'{source} {direction} S3 bucket {bucket}{target}'
+
+    return detail
+
 def log_to_console(output_dict):
     ok = '\u2714' if output_dict.get('ok') else '\u2718'
     type_ = output_dict.get('type', '')
-    stage = type_.upper().ljust(4)
+    stage = type_.upper()[:4].ljust(4)
 
     status = ''.rjust(4)
     detail = ''
     output = None
 
     if type_ in ('pull_http', 'push_http'):
-        status = str(output_dict.get('data', {}).get('status', '')).rjust(4)
-        file_ = output_dict.get('data', {}).get('file', '')
-        url = output_dict.get('data', {}).get('url', '')
-        direction = 'to' if 'push' in type_ else 'from'
-        detail = f'{file_} {direction} {url}'
+        status, detail = _http_details(output_dict)
+    elif type_ in ('pull_s3', 'push_s3'):
+        detail = _s3_details(output_dict)
     elif type_ == 'run':
         status = str(output_dict.get('data', {}).get('exit_code', '')).rjust(4)
         detail = ' '.join(output_dict.get('data', {}).get('command', []))


Test patch:
diff --git a/tst/test_log.py b/tst/test_log.py
index 596d35c..ed94950 100644
--- a/tst/test_log.py
+++ b/tst/test_log.py
@@ -10,18 +10,45 @@ class LogTest(TestCase):
         print_mock.assert_called()
 
     @patch('builtins.print')
-    def test_log_to_console_prints_type_and_status(self, print_mock):
+    def test_log_to_console_prints_logs_as_specified(self, print_mock):
+        url = 'example.com'
+        f = 'filename'
+
         testdata = [
-            ({'type': 'pull_http', 'ok': True, 'data': {'status': 200}}, ['\u2714', '200', 'PULL']),
-            ({'type': 'run', 'ok': False, 'data': {'exit_code': 200}}, ['\u2718', '200', 'RUN']),
+            # Type and status
+            ({'type': 'pull_http', 'data': {
+                'file': f,
+                'url': url,
+            }}, [f, 'from', url], []),
+            ({'type': 'push_http', 'data': {
+                'file': f,
+                'url': url,
+            }}, [f, 'to', url], []),
+            # HTTP details
+            ({'type': 'pull_http', 'ok': True, 'data': {'status': 200}}, ['\u2714', '200', 'PULL'], ['_HTTP']),
+            ({'type': 'run', 'ok': False, 'data': {'exit_code': 200}}, ['\u2718', '200', 'RUN'], ['_HTTP']),
+            # S3 details
+            ({'type': 'pull_s3', 'data': {
+                'bucket': 'b',
+                'object_name': 'a',
+                'filename': 'c',
+            }}, ['PULL', 'a from S3 bucket b as c'], ['_S3']),
+            ({'type': 'push_s3', 'data': {
+                'bucket': 'b',
+                'object_name': 'a',
+                'filename': 'a',
+            }}, ['PUSH', 'a to S3 bucket b'], ['as a', '_S3']),
         ]
 
-        for data, results in testdata:
+        for data, p_results, n_results in testdata:
             log_to_console(data)
             output = print_mock.call_args[-2][0]
 
-            for result in results:
+            for result in p_results:
                 self.assertIn(result, output)
+            for result in n_results:
+                self.assertNotIn(result, output)
+
 
     @patch('builtins.print')
     def test_log_to_console_prints_run_output(self, print_mock):
@@ -32,20 +59,3 @@ class LogTest(TestCase):
         output = print_mock.call_args[-2][0]
 
         self.assertIn(result, output)
-
-    @patch('builtins.print')
-    def test_log_to_console_prints_pull_and_push_url(self, print_mock):
-        url = 'example.com'
-        f = 'filename'
-
-        testdata = [
-            ({'type': 'pull_http', 'data': {'file': f, 'url': url}}, [f, 'from', url]),
-            ({'type': 'push_http', 'data': {'file': f, 'url': url}}, [f, 'to', url]),
-        ]
-
-        for data, results in testdata:
-            log_to_console(data)
-            output = print_mock.call_args[-2][0]
-
-            for result in results:
-                self.assertIn(result, output)