Small improvements (#45)

cli build validation, removing "missing project name" message, reading vault_secret from deployment_config

Author: beczkowb

bigflow/build.py

@@ -8,14 +8,15 @@
 
 import xmlrunner
 
-from .cli import walk_workflows, import_deployment_config, _valid_datetime
+from .cli import walk_workflows, import_deployment_config, _valid_datetime, SETUP_VALIDATION_MESSAGE
 from .dagbuilder import generate_dag_file
 from .resources import read_requirements, find_all_resources
 from .utils import resolve, now
 from .version import get_version
 from .utils import run_process
 
 
+
 __all__ = [
     'project_setup',
     'auto_configuration'
@@ -150,6 +151,7 @@ def finalize_options(self) -> None:
             pass
 
         def run(self) -> None:
+            print(SETUP_VALIDATION_MESSAGE)
             _valid_datetime(self.start_time)
             if self.build_package or self.should_run_whole_build():
                 print('Building the pip package')

bigflow/cli.py

@@ -17,6 +17,8 @@
 from bigflow.resources import find_file
 from .utils import run_process
 
+SETUP_VALIDATION_MESSAGE = 'BigFlow setup is valid.'
+
 
 def resolve(path: Path) -> str:
     return str(path.absolute())
@@ -219,8 +221,7 @@ def cli_run(project_package: str,
 
 
 def _parse_args(project_name: Optional[str], args) -> Namespace:
-    project_name_description = build_project_name_description(project_name)
-    parser = argparse.ArgumentParser(description=f'Welcome to BigFlow CLI. {project_name_description}'
+    parser = argparse.ArgumentParser(description=f'Welcome to BigFlow CLI.'
                                                   '\nType: bigflow {run,deploy-dags,deploy-image,deploy,build,build-dags,build-image,build-package} -h to print detailed help for a selected command.')
     subparsers = parser.add_subparsers(dest='operation',
                                        required=True,
@@ -455,12 +456,16 @@ def _resolve_property(args, property_name):
 
 
 def _cli_deploy_dags(args):
+    try:
+        vault_secret = _resolve_property(args, 'vault_secret')
+    except ValueError:
+        vault_secret = None
     deploy_dags_folder(dags_dir=_resolve_dags_dir(args),
                        dags_bucket=_resolve_property(args, 'dags_bucket'),
                        clear_dags_folder=args.clear_dags_folder,
                        auth_method=args.auth_method,
                        vault_endpoint=_resolve_vault_endpoint(args),
-                       vault_secret=args.vault_secret,
+                       vault_secret=vault_secret,
                        project_id=_resolve_property(args, 'gcp_project_id')
                        )
 
@@ -471,6 +476,10 @@ def _load_image_from_tar(image_tar_path: str):
 
 def _cli_deploy_image(args):
     docker_repository = _resolve_property(args, 'docker_repository')
+    try:
+        vault_secret = _resolve_property(args, 'vault_secret')
+    except ValueError:
+        vault_secret = None
     if args.image_tar_path:
         build_ver = _decode_version_number_from_file_name(Path(args.image_tar_path))
         image_id = load_image_from_tar(args.image_tar_path)
@@ -482,23 +491,23 @@ def _cli_deploy_image(args):
                         auth_method=args.auth_method,
                         docker_repository=docker_repository,
                         vault_endpoint=_resolve_vault_endpoint(args),
-                        vault_secret=args.vault_secret)
+                        vault_secret=vault_secret)
 
 
 def _cli_build_image(args):
-    check_if_project_setup_exists()
+    validate_project_setup()
     cmd = 'python project_setup.py build_project --build-image' + (' --export-image-to-file' if args.export_image_to_file else '')
     run_process(cmd)
 
 
 def _cli_build_package():
-    check_if_project_setup_exists()
+    validate_project_setup()
     cmd = 'python project_setup.py build_project --build-package'
     run_process(cmd)
 
 
 def _cli_build_dags(args):
-    check_if_project_setup_exists()
+    validate_project_setup()
     cmd = ['python', 'project_setup.py', 'build_project', '--build-dags']
     if args.workflow:
         cmd.append('--workflow')
@@ -510,7 +519,7 @@ def _cli_build_dags(args):
 
 
 def _cli_build(args):
-    check_if_project_setup_exists()
+    validate_project_setup()
     cmd = ['python', 'project_setup.py', 'build_project']
     if args.workflow:
         cmd.append('--workflow')
@@ -527,6 +536,14 @@ def check_if_project_setup_exists():
     find_file('project_setup.py', Path('.'), 1)
 
 
+def validate_project_setup():
+    check_if_project_setup_exists()
+    cmd = ['python', 'project_setup.py', 'build_project', '--validate-setup']
+    output = run_process(cmd)
+    if SETUP_VALIDATION_MESSAGE not in output:
+        raise ValueError('The project_setup.py is invalid. Check the documentation how to create a valid project_setup.py: https://github.com/allegro/bigflow/blob/master/docs/build.md')
+
+
 def cli(raw_args) -> None:
     project_name = read_project_name_from_setup()
     parsed_args = _parse_args(project_name, raw_args)

bf renamed to scripts/bf

@@ -0,0 +1,7 @@
+#!/usr/bin/env python
+
+import sys
+
+from bigflow.cli import cli
+
+cli(sys.argv[1:])

scripts/bigflow

@@ -39,7 +39,7 @@ def run(self):
 
 setuptools.setup(
     name="bigflow",
-    version="1.0.dev3",
+    version="1.0.dev4",
     author=u"Chi",
     author_email="[email protected]",
     description="BigQuery client wrapper with clean API",
@@ -58,7 +58,7 @@ def run(self):
     extras_require={
         'stackdriver': stackdriver_extras_require
     },
-    scripts=["bf"],
+    scripts=["scripts/bf", "scripts/bigflow"],
     cmdclass={
         'build_and_install_wheel': BuildAndInstallWheelCommand
     }

setup.py

@@ -3,7 +3,7 @@
 from pathlib import Path
 import subprocess
 from unittest import TestCase, mock
-from bigflow.cli import walk_module_files
+from bigflow.cli import walk_module_files, SETUP_VALIDATION_MESSAGE
 from bigflow.version import get_version
 from bigflow.build import now, get_docker_image_id, build_docker_image_tag, \
     clear_image_leftovers, clear_package_leftovers, clear_dags_leftovers, auto_configuration, \
@@ -147,6 +147,13 @@ def test_should_build_project_artifacts(self):
         self.assertFalse(image_leftovers_exist())
         self.assertFalse(dags_leftovers_exist())
 
+    def test_should_validate_project_setup(self):
+        # expected
+        self.assertTrue(SETUP_VALIDATION_MESSAGE in self.test_project.run_build('python project_setup.py build_project'))
+        self.assertTrue(SETUP_VALIDATION_MESSAGE in self.test_project.run_build('python project_setup.py build_project --build-dags'))
+        self.assertTrue(SETUP_VALIDATION_MESSAGE in self.test_project.run_build('python project_setup.py build_project --build-image'))
+        self.assertTrue(SETUP_VALIDATION_MESSAGE in self.test_project.run_build('python project_setup.py build_project --build-package'))
+
 
 class BuildPackageCommandE2E(SetupTestCase):
     def test_should_execute_build_package_command(self):

test/test_build.py

@@ -306,7 +306,8 @@ def test_should_call_cli_deploy_dags_command__with_defaults_and_with_implicit_de
 deployment_config = Config(name='dev',
                            properties={
                                'gcp_project_id': 'my-gcp-project-id',
-                               'dags_bucket': 'my-dags-bucket'
+                               'dags_bucket': 'my-dags-bucket',
+                               'vault_secret': 'secret'
                            })
         ''')
 
@@ -320,7 +321,7 @@ def test_should_call_cli_deploy_dags_command__with_defaults_and_with_implicit_de
                                                    dags_dir=self._expected_default_dags_dir(),
                                                    project_id='my-gcp-project-id',
                                                    vault_endpoint=None,
-                                                   vault_secret=None)
+                                                   vault_secret='secret')
 
         dc_file.unlink()
 
@@ -334,12 +335,14 @@ def test_should_call_cli_deploy_dags_command_for_different_environments(self, de
 deployment_config = Config(name='dev',
                           properties={
                               'gcp_project_id': 'my-gcp-dev-project-id',
-                              'dags_bucket': 'my-dags-dev-bucket'
+                              'dags_bucket': 'my-dags-dev-bucket',
+                              'vault_secret': 'secret-dev'
                           })\
     .add_configuration(name='prod', 
                           properties={
                               'gcp_project_id': 'my-gcp-prod-project-id',
-                              'dags_bucket': 'my-dags-prod-bucket'
+                              'dags_bucket': 'my-dags-prod-bucket',
+                              'vault_secret': 'secret-prod'
                           })                          
                           
         ''')
@@ -354,7 +357,7 @@ def test_should_call_cli_deploy_dags_command_for_different_environments(self, de
                                                    dags_dir=self._expected_default_dags_dir(),
                                                    project_id='my-gcp-dev-project-id',
                                                    vault_endpoint=None,
-                                                   vault_secret=None)
+                                                   vault_secret='secret-dev')
 
         # when
         cli(['deploy-dags', '--config', 'dev'])
@@ -366,7 +369,7 @@ def test_should_call_cli_deploy_dags_command_for_different_environments(self, de
                                                    dags_dir=self._expected_default_dags_dir(),
                                                    project_id='my-gcp-dev-project-id',
                                                    vault_endpoint=None,
-                                                   vault_secret=None)
+                                                   vault_secret='secret-dev')
 
         # when
         cli(['deploy-dags', '--config', 'prod'])
@@ -378,7 +381,7 @@ def test_should_call_cli_deploy_dags_command_for_different_environments(self, de
                                                    dags_dir=self._expected_default_dags_dir(),
                                                    project_id='my-gcp-prod-project-id',
                                                    vault_endpoint=None,
-                                                   vault_secret=None)
+                                                   vault_secret='secret-prod')
 
         dc_file.unlink()
 
@@ -393,16 +396,16 @@ def test_should_call_cli_deploy_dags_command__when_parameters_are_given_by_expli
                         properties={
                                'gcp_project_id': 'my-another-gcp-project-id',
                                'vault_endpoint': 'my-another-vault-endpoint',
-                               'dags_bucket': 'my-another-dags-bucket'
+                               'dags_bucket': 'my-another-dags-bucket',
+                               'vault_secret': 'secrett'
                         })
         ''')
 
         #when
         cli(['deploy-dags',
              '--deployment-config-path', dc_file.as_posix(),
              '--dags-dir', '/tmp/my-dags-dir',
-             '--auth-method', 'service_account',
-             '--vault-secret', 'secrett'
+             '--auth-method', 'service_account'
             ])
 
         #then
@@ -539,8 +542,6 @@ def test_should_call_both_deploy_methods_with_deploy_command(self, deploy_docker
         # when
         cli(['deploy', '-v', '0.0.2'])
 
-
-
         # then
         deploy_dags_folder_mock.assert_called_with(auth_method='local_account',
                                                    clear_dags_folder=False,
@@ -608,6 +609,20 @@ def test_should_call_cli_build_image_command(self, _cli_build_image_mock):
         # then
         _cli_build_image_mock.assert_called_with(Namespace(operation='build-image', export_image_to_file=True))
 
+    @mock.patch('bigflow.cli.run_process')
+    @mock.patch('bigflow.cli.validate_project_setup')
+    def test_should_call_build_command_through_CLI(
+            self, validate_project_setup_mock, run_process_mock):
+        # given
+        validate_project_setup_mock.return_value = '.'
+
+        # when
+        cli(['build'])
+
+        # then
+        self.assertEqual(run_process_mock.call_count, 1)
+        run_process_mock.assert_any_call('python project_setup.py build_project'.split(' '))
+
     @mock.patch('bigflow.cli._cli_build_package')
     def test_should_call_cli_build_package_command(self, _cli_build_package_mock):
         # when
@@ -645,58 +660,50 @@ def test_should_call_cli_build_command(self, _cli_build_mock):
         _cli_build_mock.assert_called_with(Namespace(operation='build', export_image_to_file=True,
                                                      start_time='2020-01-01 00:00:00', workflow='some_workflow'))
 
-    @mock.patch('bigflow.cli.check_if_project_setup_exists')
     @mock.patch('bigflow.cli.run_process')
-    def test_should_call_build_command_through_CLI(self, run_process_mock, check_if_project_setup_exists_mock):
-        # given
-        check_if_project_setup_exists_mock.return_value = TEST_PROJECT_PATH
-
-        # when
-        cli(['build'])
-
-        # then
-        self.assertEqual(run_process_mock.call_count, 1)
-        run_process_mock.assert_any_call('python project_setup.py build_project --build-dags --build-image --build-package'.format(str(TEST_PROJECT_PATH)))
-
-    @mock.patch('bigflow.cli.check_if_project_setup_exists')
-    @mock.patch('bigflow.cli.run_process')
-    def test_should_call_build_package_command_through_CLI(self, run_process_mock, check_if_project_setup_exists_mock):
-        # given
-        check_if_project_setup_exists_mock.return_value = TEST_PROJECT_PATH
-
+    @mock.patch('bigflow.cli.validate_project_setup')
+    def test_should_call_build_package_command_through_CLI(self, validate_project_setup_mock, run_process_mock):
         # when
         cli(['build-package'])
 
         # then
         self.assertEqual(run_process_mock.call_count, 1)
         run_process_mock.assert_any_call('python project_setup.py build_project --build-package')
 
-    @mock.patch('bigflow.cli.check_if_project_setup_exists')
     @mock.patch('bigflow.cli.run_process')
-    def test_should_call_build_command_through_CLI(self, run_process_mock, check_if_project_setup_exists_mock):
-        # given
-        check_if_project_setup_exists_mock.return_value = TEST_PROJECT_PATH
-
+    @mock.patch('bigflow.cli.find_file')
+    @mock.patch('bigflow.cli.validate_project_setup')
+    def test_should_call_build_image_command_through_CLI(self, validate_project_setup_mock, find_file_mock, run_process_mock):
         # when
         cli(['build-image'])
 
         # then
         self.assertEqual(run_process_mock.call_count, 1)
         run_process_mock.assert_any_call('python project_setup.py build_project --build-image')
 
-    @mock.patch('bigflow.cli.check_if_project_setup_exists')
     @mock.patch('bigflow.cli.run_process')
-    def test_should_call_build_dags_command_through_CLI(self, run_process_mock, check_if_project_setup_exists_mock):
-        # given
-        check_if_project_setup_exists_mock.return_value = TEST_PROJECT_PATH
-
+    @mock.patch('bigflow.cli.validate_project_setup')
+    def test_should_call_build_dags_command_through_CLI(self, validate_project_setup_mock, run_process_mock):
         # when
         cli(['build-dags'])
 
         # then
         self.assertEqual(run_process_mock.call_count, 1)
         run_process_mock.assert_any_call('python project_setup.py build_project --build-dags'.split(' '))
 
+    @mock.patch('bigflow.cli.run_process')
+    @mock.patch('bigflow.cli.validate_project_setup')
+    def test_should_validate_project_setup_before_build(
+            self, validate_project_setup_mock, run_process_mock):
+        # when
+        cli(['build'])
+        cli(['build-image'])
+        cli(['build-dags'])
+        cli(['build-package'])
+
+        # then
+        self.assertEqual(validate_project_setup_mock.call_count, 4)
+
     def _expected_default_dags_dir(self):
         return (Path(os.getcwd()) / '.dags').as_posix()
 
@@ -707,3 +714,17 @@ def _touch_file(self, file_name: str, content: str = ''):
         f.write_text(content)
         return f
 
+
+class ValidateProjectSetupTestCase(TestCase):
+
+    @mock.patch('bigflow.cli.check_if_project_setup_exists')
+    @mock.patch('bigflow.cli.run_process')
+    def test_should_raise_error_if_no_expected_message_found_in_setup_output(
+            self, run_process_mock, check_if_project_setup_exists_mock):
+        # given
+        run_process_mock.return_value = 'Unexpected message'
+
+        # then
+        with self.assertRaises(ValueError) as e:
+            # when
+            validate_project_setup()