From 9b243e4ac0b3f620c8d5456d638f3e34b81def8f Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Mon, 11 Sep 2023 14:43:57 +0800 Subject: [PATCH 1/9] Support compare by provide commits Signed-off-by: Wei-Chun, Chang --- piperider_cli/cli.py | 18 ++++- .../cli_utils/compare_with_recipe.py | 2 + piperider_cli/dbt/list_task.py | 8 +- piperider_cli/dbtutil.py | 10 ++- piperider_cli/recipe_executor.py | 45 ++++++------ piperider_cli/recipes/__init__.py | 73 ++++++++++++++----- .../recipes/default_recipe_generator.py | 9 +++ piperider_cli/recipes/utils.py | 8 +- 8 files changed, 118 insertions(+), 55 deletions(-) diff --git a/piperider_cli/cli.py b/piperider_cli/cli.py index d71142734..0f698bf36 100644 --- a/piperider_cli/cli.py +++ b/piperider_cli/cli.py @@ -430,6 +430,7 @@ def cloud_compare_reports(**kwargs): @cli.command(name='compare', short_help='Compare the change for the current branch.', cls=TrackCommand) +@click.argument('commits', nargs=-1, type=click.STRING) @click.option('--recipe', default=None, type=click.STRING, help='Select a different recipe.') @click.option('--upload', default=False, is_flag=True, help='Upload the report to PipeRider Cloud.') @click.option('--share', default=False, is_flag=True, help='Enable public share of the report to PipeRider Cloud.') @@ -450,11 +451,26 @@ def cloud_compare_reports(**kwargs): ]) @add_options(dbt_related_options) @add_options(debug_option) -def compare_with_recipe(**kwargs): +def compare_with_recipe(commits, **kwargs): """ Generate the comparison report for your branch. """ + console = Console() + if len(commits) > 1: + console.print('[bold red]Error:[/bold red] Commit format is not supported') + sys.exit(1) + elif len(commits) == 1: + commits = commits[0] + if '...' in commits: + kwargs['base_branch'] = commits.split('...')[0] + kwargs['target_branch'] = commits.split('...')[1] + elif '..' in commits: + console.print('[bold red]Error:[/bold red] Two-dot diff comparisons are not supported') + sys.exit(1) + else: + kwargs['base_branch'] = commits + from piperider_cli.cli_utils.compare_with_recipe import compare_with_recipe as cmd return cmd(**kwargs) diff --git a/piperider_cli/cli_utils/compare_with_recipe.py b/piperider_cli/cli_utils/compare_with_recipe.py index 05116d390..81e697d09 100644 --- a/piperider_cli/cli_utils/compare_with_recipe.py +++ b/piperider_cli/cli_utils/compare_with_recipe.py @@ -21,6 +21,7 @@ def compare_with_recipe(**kwargs): modified = kwargs.get('modified') base_branch = kwargs.get('base_branch') + target_branch = kwargs.get('target_branch') skip_datasource_connection = kwargs.get('skip_datasource') # reconfigure recipe global flags @@ -56,6 +57,7 @@ def compare_with_recipe(**kwargs): select=select, modified=modified, base_branch=base_branch, + target_branch=target_branch, skip_datasource_connection=skip_datasource_connection, debug=debug) last = False diff --git a/piperider_cli/dbt/list_task.py b/piperider_cli/dbt/list_task.py index 0e1322f79..b2b877a44 100644 --- a/piperider_cli/dbt/list_task.py +++ b/piperider_cli/dbt/list_task.py @@ -26,11 +26,11 @@ def create_temp_dir(): return tempfile.mkdtemp() -def load_full_manifest(target_path: str): +def load_full_manifest(target_path: str, project_dir: str = None): from dbt.adapters.factory import register_adapter from dbt.parser.manifest import ManifestLoader - runtime_config = PrepareRuntimeConfig(target_path) + runtime_config = PrepareRuntimeConfig(target_path, project_dir=project_dir) register_adapter(runtime_config) v = dbt_version @@ -207,9 +207,9 @@ def convert_time_type(cls, agate_table: agate.Table, col_idx: int) -> str: pass -def PrepareRuntimeConfig(target_path: str): +def PrepareRuntimeConfig(target_path: str, project_dir: str = None): from piperider_cli.configuration import FileSystem - project_root = FileSystem.WORKING_DIRECTORY + project_root = project_dir if project_dir is not None else FileSystem.WORKING_DIRECTORY profiles_dir = FileSystem.DBT_PROFILES_DIR def _get_v13_runtime_config(flags): diff --git a/piperider_cli/dbtutil.py b/piperider_cli/dbtutil.py index 802a8247e..b78c15fb9 100644 --- a/piperider_cli/dbtutil.py +++ b/piperider_cli/dbtutil.py @@ -134,9 +134,11 @@ def _get_state_run_results(dbt_state_dir: str): return run_results -def _get_state_manifest(dbt_state_dir: str): +def _get_state_manifest(dbt_state_dir: str, project_dir: str = None): path = os.path.join(dbt_state_dir, 'manifest.json') - if os.path.isabs(path) is False: + if project_dir is not None: + path = os.path.join(project_dir, path) + elif os.path.isabs(path) is False: from piperider_cli.configuration import FileSystem path = os.path.join(FileSystem.WORKING_DIRECTORY, path) with open(path) as f: @@ -656,8 +658,8 @@ def check_dbt_manifest(dbt_state_dir: str) -> bool: return os.path.exists(path) -def get_dbt_manifest(dbt_state_dir: str): - return _get_state_manifest(dbt_state_dir) +def get_dbt_manifest(dbt_state_dir: str, project_dir: str = None): + return _get_state_manifest(dbt_state_dir, project_dir=project_dir) def load_dbt_resources(target_path: str, select: tuple = None, state=None): diff --git a/piperider_cli/recipe_executor.py b/piperider_cli/recipe_executor.py index a8d580c30..b7b8d56ff 100644 --- a/piperider_cli/recipe_executor.py +++ b/piperider_cli/recipe_executor.py @@ -12,8 +12,8 @@ class RecipeExecutor: @staticmethod - def exec(recipe_name: str, auto_generate_default_recipe: bool = True, select: tuple = None, modified: bool = False, - base_branch: str = None, skip_datasource_connection: bool = False, debug=False) -> RecipeConfiguration: + def exec(recipe_name: str, select: tuple = None, modified: bool = False, base_branch: str = None, + target_branch: str = None, skip_datasource_connection: bool = False, debug=False) -> RecipeConfiguration: config = Configuration.instance() recipe_path = select_recipe_file(recipe_name) @@ -27,26 +27,27 @@ def exec(recipe_name: str, auto_generate_default_recipe: bool = True, select: tu console.print( f"[[bold green]Select[/bold green]] Manually select the dbt nodes to run by '{','.join(select)}'") if recipe_path is None or select or modified or base_branch or skip_datasource_connection: - if auto_generate_default_recipe: - dbt_project_path = None - if config.dataSources and config.dataSources[0].args.get('dbt'): - dbt_project_path = os.path.relpath(config.dataSources[0].args.get('dbt', {}).get('projectDir')) - # generate a default recipe - console.rule("Recipe executor: generate recipe") - options = dict(base_branch=base_branch, skip_datasource_connection=skip_datasource_connection) - if select: - options['select'] = select - recipe = generate_default_recipe(overwrite_existing=False, - dbt_project_path=dbt_project_path, - options=options) - if recipe is None: - raise RecipeConfigException( - message='Default recipe generation failed.', - hint='Please provide a recipe file or feedback an issue to us' - ) - show_recipe_content(recipe) - else: - raise FileNotFoundError(f"Cannot find the recipe '{recipe_name}'") + dbt_project_path = None + if config.dataSources and config.dataSources[0].args.get('dbt'): + dbt_project_path = os.path.relpath(config.dataSources[0].args.get('dbt', {}).get('projectDir')) + # generate a default recipe + console.rule("Recipe executor: generate recipe") + options = dict( + base_branch=base_branch, + target_branch=target_branch, + skip_datasource_connection=skip_datasource_connection + ) + if select: + options['select'] = select + recipe = generate_default_recipe(overwrite_existing=False, + dbt_project_path=dbt_project_path, + options=options) + if recipe is None: + raise RecipeConfigException( + message='Default recipe generation failed.', + hint='Please provide a recipe file or feedback an issue to us' + ) + show_recipe_content(recipe) else: recipe = RecipeConfiguration.load(recipe_path) diff --git a/piperider_cli/recipes/__init__.py b/piperider_cli/recipes/__init__.py index 2763c0fa1..38919a770 100644 --- a/piperider_cli/recipes/__init__.py +++ b/piperider_cli/recipes/__init__.py @@ -279,36 +279,45 @@ def prepare_dbt_resources_candidate(cfg: RecipeConfiguration, options: Dict): select = () if cfg.auto_generated: select = update_select_with_cli_option(options) - dbt_project = dbtutil.load_dbt_project(config.dbt.get('projectDir')) - target_path = dbt_project.get('target-path') if dbt_project.get('target-path') else 'target' if require_base_state(cfg): - execute_dbt_compile_archive(cfg.base) + execute_dbt_compile_archive(cfg, recipe_type='base') state = cfg.base.state_path - # elif dbtutil.check_dbt_manifest(target_path) is False: - # Need to compile the dbt project if the manifest file does not exist - execute_dbt_deps(cfg.base) - execute_dbt_compile(cfg.base) + + if cfg.target.branch is not None: + execute_dbt_compile_archive(cfg, recipe_type='target') + target_path = 'state' + else: + execute_dbt_deps(cfg.target) + execute_dbt_compile(cfg.target) + dbt_project = dbtutil.load_dbt_project(config.dbt.get('projectDir')) + target_path = dbt_project.get('target-path') if dbt_project.get('target-path') else 'target' if any('state:' in item for item in select): console.print(f"Run: \[dbt list] select option '{' '.join(select)}' with state") - resources = tool().list_dbt_resources(target_path, select=select, state=state) + resources = tool().list_dbt_resources( + target_path, project_dir=cfg.target.tmp_dir_path, select=select, state=state + ) elif options.get('skip_datasource_connection'): console.print("Skip: \[dbt list] due to the command option '--skip-datasource'") resources = [] else: console.print(f"Run: \[dbt list] select option '{' '.join(select)}'") - resources = tool().list_dbt_resources(target_path, select=select) + resources = tool().list_dbt_resources(target_path, project_dir=cfg.target.tmp_dir_path, select=select) console.print() return resources, state -def execute_recipe(model: RecipeModel, debug=False, recipe_type='base'): +def execute_recipe(cfg: RecipeConfiguration, recipe_type='base', debug=False): """ We execute a recipe in the following steps: 1. run dbt commands 2. run piperider commands """ + if recipe_type == 'target': + model = cfg.target + else: + model = cfg.base if model.is_file_specified(): console.print(f"Select {recipe_type} report: \[{model.file}]") @@ -339,8 +348,18 @@ def execute_recipe(model: RecipeModel, debug=False, recipe_type='base'): console.print() -def execute_dbt_compile_archive(model: RecipeModel, debug=False): - branch_or_commit = tool().git_merge_base(model.branch, 'HEAD') or model.branch +def execute_dbt_compile_archive(cfg: RecipeConfiguration, recipe_type='base'): + if recipe_type == 'target': + model = cfg.target + else: + model = cfg.base + + if recipe_type == 'target': + branch_or_commit = cfg.target.branch + else: + diff_target = cfg.target.branch if cfg.target.branch else 'HEAD' + branch_or_commit = tool().git_merge_base(cfg.base.branch, diff_target) or cfg.base.branch + if not branch_or_commit: raise RecipeConfigException("Branch is not specified") @@ -350,7 +369,6 @@ def execute_dbt_compile_archive(model: RecipeModel, debug=False): execute_dbt_deps(model, model.tmp_dir_path, Path(FileSystem.DBT_PROFILES_DIR).as_posix()) execute_dbt_compile(model, model.tmp_dir_path, Path(FileSystem.DBT_PROFILES_DIR).as_posix(), model.state_path) - pass def execute_dbt_deps(model: RecipeModel, project_dir: str = None, profiles_dir: str = None): @@ -366,7 +384,6 @@ def execute_dbt_deps(model: RecipeModel, project_dir: str = None, profiles_dir: f"[bold yellow]Warning: [/bold yellow] Dbt command failed: '{cmd}' with exit code: {exit_code}") sys.exit(exit_code) console.print() - pass def execute_dbt_compile(model: RecipeModel, project_dir: str = None, profiles_dir: str = None, @@ -385,10 +402,9 @@ def execute_dbt_compile(model: RecipeModel, project_dir: str = None, profiles_di f"[bold yellow]Warning: [/bold yellow] Dbt command failed: '{cmd}' with exit code: {exit_code}") sys.exit(exit_code) console.print() - pass -def execute_recipe_archive(model: RecipeModel, debug=False, recipe_type='base'): +def execute_recipe_archive(cfg: RecipeConfiguration, recipe_type='base', debug=False): """ We execute a recipe in the following steps: 1. export the repo with specified commit or branch if needed @@ -396,13 +412,27 @@ def execute_recipe_archive(model: RecipeModel, debug=False, recipe_type='base'): 3. run piperider commands """ + if recipe_type == 'target': + model = cfg.target + else: + model = cfg.base + if model.is_file_specified(): console.print(f"Select {recipe_type} report: \[{model.file}]") return - branch_or_commit = tool().git_merge_base(model.branch, 'HEAD') or model.branch + diff_target = 'HEAD' + if recipe_type == 'target': + branch_or_commit = cfg.target.branch + else: + diff_target = cfg.target.branch if cfg.target.branch else 'HEAD' + branch_or_commit = tool().git_merge_base(cfg.base.branch, diff_target) or cfg.base.branch + if branch_or_commit: - console.print(f"Run: \[git archive] {model.branch}...HEAD = {branch_or_commit}") + if recipe_type == 'target': + console.print(f"Run: \[git archive] {model.branch}") + else: + console.print(f"Run: \[git archive] {model.branch}...{diff_target} = {branch_or_commit}") if model.tmp_dir_path is None: model.tmp_dir_path = tool().git_archive(branch_or_commit) # NOTE: Passing git branch information through environment variables, @@ -496,10 +526,13 @@ def execute_recipe_configuration(cfg: RecipeConfiguration, options, debug=False) cfg.target.dbt.commands = replace_commands_dbt_state_path(cfg.target.dbt.commands, dbt_state_path) console.rule("Recipe executor: base phase") - execute_recipe_archive(cfg.base, recipe_type='base', debug=debug) + execute_recipe_archive(cfg, recipe_type='base', debug=debug) console.rule("Recipe executor: target phase") - execute_recipe(cfg.target, recipe_type='target', debug=debug) + if cfg.target.branch: + execute_recipe_archive(cfg, recipe_type='target', debug=debug) + else: + execute_recipe(cfg, recipe_type='target', debug=debug) except Exception as e: if isinstance(e, InteractiveStopException): diff --git a/piperider_cli/recipes/default_recipe_generator.py b/piperider_cli/recipes/default_recipe_generator.py index 1f53c9387..0b4ff7e25 100644 --- a/piperider_cli/recipes/default_recipe_generator.py +++ b/piperider_cli/recipes/default_recipe_generator.py @@ -89,6 +89,15 @@ def _create_target_recipe(dbt_project_path=None, options: dict = None) -> Recipe """ target = RecipeModel() + if tool().git_branch() is not None: + if options.get('target_branch') is not None: + if tool().git_branch(options.get('target_branch')) is not None: + target.branch = options.get('target_branch') + else: + ex = RecipeException(f"Cannot find specified base branch: {options.get('target_branch')}") + ex.hint = f"Please check if the specified branch name '{options.get('target_branch')}' is correct." + raise ex + dbt_project = _read_dbt_project_file(dbt_project_path) if dbt_project: target.dbt = _prepare_dbt_cmds(options, target=True) diff --git a/piperider_cli/recipes/utils.py b/piperider_cli/recipes/utils.py index 9b38b3e78..dd89ff4f7 100644 --- a/piperider_cli/recipes/utils.py +++ b/piperider_cli/recipes/utils.py @@ -162,11 +162,11 @@ def check_dbt_command(self): if exit_code != 0: raise PipeRiderError("dbt is not installed", hint="Please install it first") - def list_dbt_resources(self, target_path, select=None, state=None): + def list_dbt_resources(self, target_path, project_dir=None, select=None, state=None): if state: - manifest = load_full_manifest(target_path) + manifest = load_full_manifest(target_path, project_dir=project_dir) else: - manifest = load_manifest(get_dbt_manifest(target_path)) + manifest = load_manifest(get_dbt_manifest(target_path, project_dir=project_dir)) return list_resources_unique_id_from_manifest(manifest, select=select, state=state) @@ -194,7 +194,7 @@ def git_archive(self, commit_or_branch): self.console.print(f"[green]:external-command:>[/green] [default]{cmd}[/default]") return '/path/to/tmp' - def list_dbt_resources(self, target_path, select=None, state=None): + def list_dbt_resources(self, target_path, project_dir=None, select=None, state=None): state_msg = '' if state: state_msg = f'state: {state}' From 22f8679cf9d0940ca9d91c63f648df009d550802 Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Thu, 14 Sep 2023 14:05:25 +0800 Subject: [PATCH 2/9] Add git rev-parse for getting commits Signed-off-by: Wei-Chun, Chang --- piperider_cli/recipes/__init__.py | 4 ++-- piperider_cli/recipes/utils.py | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/piperider_cli/recipes/__init__.py b/piperider_cli/recipes/__init__.py index 38919a770..0bbb5258c 100644 --- a/piperider_cli/recipes/__init__.py +++ b/piperider_cli/recipes/__init__.py @@ -355,7 +355,7 @@ def execute_dbt_compile_archive(cfg: RecipeConfiguration, recipe_type='base'): model = cfg.base if recipe_type == 'target': - branch_or_commit = cfg.target.branch + branch_or_commit = tool().git_rev_parse(cfg.target.branch) else: diff_target = cfg.target.branch if cfg.target.branch else 'HEAD' branch_or_commit = tool().git_merge_base(cfg.base.branch, diff_target) or cfg.base.branch @@ -423,7 +423,7 @@ def execute_recipe_archive(cfg: RecipeConfiguration, recipe_type='base', debug=F diff_target = 'HEAD' if recipe_type == 'target': - branch_or_commit = cfg.target.branch + branch_or_commit = tool().git_rev_parse(cfg.target.branch) else: diff_target = cfg.target.branch if cfg.target.branch else 'HEAD' branch_or_commit = tool().git_merge_base(cfg.base.branch, diff_target) or cfg.base.branch diff --git a/piperider_cli/recipes/utils.py b/piperider_cli/recipes/utils.py index dd89ff4f7..e20424385 100644 --- a/piperider_cli/recipes/utils.py +++ b/piperider_cli/recipes/utils.py @@ -97,6 +97,15 @@ def git_branch(self, obj_name: str = 'HEAD'): return outs + def git_rev_parse(self, obj_name: str = 'HEAD'): + outs, errs, exit_code = self.dryrun_ignored_execute_command( + f"git rev-parse {obj_name}" + ) + if exit_code != 0: + return None + + return outs + def git_checkout_to(self, commit_or_branch): outs, errs, exit_code = self.execute_command_in_silent( f"git checkout {commit_or_branch}" From 7ec4555c6a386eab07d349cebbefb47903c20820 Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Thu, 14 Sep 2023 15:17:43 +0800 Subject: [PATCH 3/9] Rename recipe branch to recipe ref Signed-off-by: Wei-Chun, Chang --- piperider_cli/cli.py | 10 +++-- .../cli_utils/compare_with_recipe.py | 8 ++-- piperider_cli/recipe_executor.py | 12 +++--- piperider_cli/recipes/__init__.py | 40 +++++++++---------- .../recipes/default_recipe_generator.py | 24 +++++------ piperider_cli/recipes/github_action.py | 12 +++--- 6 files changed, 55 insertions(+), 51 deletions(-) diff --git a/piperider_cli/cli.py b/piperider_cli/cli.py index 0f698bf36..b7a5bc91a 100644 --- a/piperider_cli/cli.py +++ b/piperider_cli/cli.py @@ -463,13 +463,17 @@ def compare_with_recipe(commits, **kwargs): elif len(commits) == 1: commits = commits[0] if '...' in commits: - kwargs['base_branch'] = commits.split('...')[0] - kwargs['target_branch'] = commits.split('...')[1] + kwargs['base_ref'] = commits.split('...')[0] + kwargs['target_ref'] = commits.split('...')[1] elif '..' in commits: console.print('[bold red]Error:[/bold red] Two-dot diff comparisons are not supported') sys.exit(1) else: - kwargs['base_branch'] = commits + kwargs['base_ref'] = commits + + if kwargs.get('base_branch') and kwargs.get('base_ref'): + console.print("[bold red]Error:[/bold red] '--base-branch' option and commit argument cannot be used together") + sys.exit(1) from piperider_cli.cli_utils.compare_with_recipe import compare_with_recipe as cmd return cmd(**kwargs) diff --git a/piperider_cli/cli_utils/compare_with_recipe.py b/piperider_cli/cli_utils/compare_with_recipe.py index 81e697d09..f08365498 100644 --- a/piperider_cli/cli_utils/compare_with_recipe.py +++ b/piperider_cli/cli_utils/compare_with_recipe.py @@ -20,8 +20,8 @@ def compare_with_recipe(**kwargs): select = kwargs.get('select') modified = kwargs.get('modified') - base_branch = kwargs.get('base_branch') - target_branch = kwargs.get('target_branch') + base_ref = kwargs.get('base_branch') if kwargs.get('base_branch') else kwargs.get('base_ref') + target_ref = kwargs.get('target_ref') skip_datasource_connection = kwargs.get('skip_datasource') # reconfigure recipe global flags @@ -56,8 +56,8 @@ def compare_with_recipe(**kwargs): recipe_name=recipe, select=select, modified=modified, - base_branch=base_branch, - target_branch=target_branch, + base_ref=base_ref, + target_ref=target_ref, skip_datasource_connection=skip_datasource_connection, debug=debug) last = False diff --git a/piperider_cli/recipe_executor.py b/piperider_cli/recipe_executor.py index b7b8d56ff..b679e4868 100644 --- a/piperider_cli/recipe_executor.py +++ b/piperider_cli/recipe_executor.py @@ -12,12 +12,12 @@ class RecipeExecutor: @staticmethod - def exec(recipe_name: str, select: tuple = None, modified: bool = False, base_branch: str = None, - target_branch: str = None, skip_datasource_connection: bool = False, debug=False) -> RecipeConfiguration: + def exec(recipe_name: str, select: tuple = None, modified: bool = False, base_ref: str = None, + target_ref: str = None, skip_datasource_connection: bool = False, debug=False) -> RecipeConfiguration: config = Configuration.instance() recipe_path = select_recipe_file(recipe_name) - if recipe_name and (select or modified or base_branch or skip_datasource_connection): + if recipe_name and (select or modified or base_ref or skip_datasource_connection): console.print( "[[bold yellow]Warning[/bold yellow]] " "The recipe will be ignored when '--select', '--modified', '--base-branch', " @@ -26,15 +26,15 @@ def exec(recipe_name: str, select: tuple = None, modified: bool = False, base_br if not skip_datasource_connection and select: console.print( f"[[bold green]Select[/bold green]] Manually select the dbt nodes to run by '{','.join(select)}'") - if recipe_path is None or select or modified or base_branch or skip_datasource_connection: + if recipe_path is None or select or modified or base_ref or skip_datasource_connection: dbt_project_path = None if config.dataSources and config.dataSources[0].args.get('dbt'): dbt_project_path = os.path.relpath(config.dataSources[0].args.get('dbt', {}).get('projectDir')) # generate a default recipe console.rule("Recipe executor: generate recipe") options = dict( - base_branch=base_branch, - target_branch=target_branch, + base_ref=base_ref, + target_ref=target_ref, skip_datasource_connection=skip_datasource_connection ) if select: diff --git a/piperider_cli/recipes/__init__.py b/piperider_cli/recipes/__init__.py index 0bbb5258c..430319163 100644 --- a/piperider_cli/recipes/__init__.py +++ b/piperider_cli/recipes/__init__.py @@ -95,7 +95,7 @@ def __dict__(self): class RecipeModel: - branch: str = None + ref: str = None dbt: RecipeDbtField = None piperider: RecipePiperiderField = None cloud: RecipeCloudField = None @@ -107,7 +107,7 @@ def __init__(self, content: dict = None): return # git branch name - self.branch: str = content.get('branch') + self.ref: str = content.get('branch') if content.get('branch') is not None else content.get('ref') if content.get('file') is not None: self.file: str = content.get('file') self.dbt: RecipeDbtField = RecipeDbtField(content.get('dbt')) @@ -116,8 +116,8 @@ def __init__(self, content: dict = None): def __dict__(self): d = dict() - if self.branch: - d['branch'] = self.branch + if self.ref: + d['ref'] = self.ref if self.is_file_specified(): d['file'] = self.file if self.dbt: @@ -131,15 +131,15 @@ def __dict__(self): def is_file_specified(self): return hasattr(self, 'file') - def is_branch_specified(self): - return self.branch is not None + def is_ref_specified(self): + return self.ref is not None def is_piperider_commands_specified(self): return len(self.piperider.commands) > 0 def validate_recipe(self): # check conflict - if (self.is_branch_specified() or self.is_piperider_commands_specified()) and self.is_file_specified(): + if (self.is_ref_specified() or self.is_piperider_commands_specified()) and self.is_file_specified(): raise RecipeConfigException( message="Both 'file' and 'branch/piperider commands' are specified.", hint="Please modify the recipe file to use either 'file' or 'branch/piperider commands'.") @@ -215,7 +215,7 @@ def load(cls, path: str) -> 'RecipeConfiguration': def verify_git_dependencies(cfg: RecipeConfiguration): - if cfg.base.branch is None and cfg.target.branch is None: + if cfg.base.ref is None and cfg.target.ref is None: # nobody set the git branch, skip the verification return @@ -284,7 +284,7 @@ def prepare_dbt_resources_candidate(cfg: RecipeConfiguration, options: Dict): execute_dbt_compile_archive(cfg, recipe_type='base') state = cfg.base.state_path - if cfg.target.branch is not None: + if cfg.target.ref is not None: execute_dbt_compile_archive(cfg, recipe_type='target') target_path = 'state' else: @@ -355,10 +355,10 @@ def execute_dbt_compile_archive(cfg: RecipeConfiguration, recipe_type='base'): model = cfg.base if recipe_type == 'target': - branch_or_commit = tool().git_rev_parse(cfg.target.branch) + branch_or_commit = tool().git_rev_parse(cfg.target.ref) else: - diff_target = cfg.target.branch if cfg.target.branch else 'HEAD' - branch_or_commit = tool().git_merge_base(cfg.base.branch, diff_target) or cfg.base.branch + diff_target = cfg.target.ref if cfg.target.ref else 'HEAD' + branch_or_commit = tool().git_merge_base(cfg.base.ref, diff_target) or cfg.base.ref if not branch_or_commit: raise RecipeConfigException("Branch is not specified") @@ -423,21 +423,21 @@ def execute_recipe_archive(cfg: RecipeConfiguration, recipe_type='base', debug=F diff_target = 'HEAD' if recipe_type == 'target': - branch_or_commit = tool().git_rev_parse(cfg.target.branch) + branch_or_commit = tool().git_rev_parse(cfg.target.ref) else: - diff_target = cfg.target.branch if cfg.target.branch else 'HEAD' - branch_or_commit = tool().git_merge_base(cfg.base.branch, diff_target) or cfg.base.branch + diff_target = cfg.target.ref if cfg.target.ref else 'HEAD' + branch_or_commit = tool().git_merge_base(cfg.base.ref, diff_target) or cfg.base.ref if branch_or_commit: if recipe_type == 'target': - console.print(f"Run: \[git archive] {model.branch}") + console.print(f"Run: \[git archive] {model.ref}") else: - console.print(f"Run: \[git archive] {model.branch}...{diff_target} = {branch_or_commit}") + console.print(f"Run: \[git archive] {model.ref}...{diff_target} = {branch_or_commit}") if model.tmp_dir_path is None: model.tmp_dir_path = tool().git_archive(branch_or_commit) # NOTE: Passing git branch information through environment variables, # as the archived folder does not have a .git folder. - model.piperider.environments['PIPERIDER_GIT_BRANCH'] = model.branch + model.piperider.environments['PIPERIDER_GIT_BRANCH'] = model.ref model.piperider.environments['PIPERIDER_GIT_SHA'] = branch_or_commit console.print() @@ -474,7 +474,7 @@ def get_current_branch(cfg: RecipeConfiguration): Update the effective branch name for cfg and return the original branch before execution """ - if cfg.base.branch is None and cfg.target.branch is None: + if cfg.base.ref is None and cfg.target.ref is None: # We don't care the current branch, because we won't change it return None @@ -529,7 +529,7 @@ def execute_recipe_configuration(cfg: RecipeConfiguration, options, debug=False) execute_recipe_archive(cfg, recipe_type='base', debug=debug) console.rule("Recipe executor: target phase") - if cfg.target.branch: + if cfg.target.ref: execute_recipe_archive(cfg, recipe_type='target', debug=debug) else: execute_recipe(cfg, recipe_type='target', debug=debug) diff --git a/piperider_cli/recipes/default_recipe_generator.py b/piperider_cli/recipes/default_recipe_generator.py index 0b4ff7e25..c516aa599 100644 --- a/piperider_cli/recipes/default_recipe_generator.py +++ b/piperider_cli/recipes/default_recipe_generator.py @@ -58,18 +58,18 @@ def _create_base_recipe(dbt_project_path=None, options: dict = None) -> RecipeMo base = RecipeModel() if tool().git_branch() is not None: - if options.get('base_branch') is not None: - if tool().git_branch(options.get('base_branch')) is not None: - base.branch = options.get('base_branch') + if options.get('base_ref') is not None: + if tool().git_branch(options.get('base_ref')) is not None: + base.ref = options.get('base_ref') else: - ex = RecipeException(f"Cannot find specified base branch: {options.get('base_branch')}") - ex.hint = f"Please check if the specified branch name '{options.get('base_branch')}' is correct." + ex = RecipeException(f"Cannot find specified base branch: {options.get('base_ref')}") + ex.hint = f"Please check if the specified branch name '{options.get('base_ref')}' is correct." raise ex else: if tool().git_branch('main') is not None: - base.branch = 'main' + base.ref = 'main' elif tool().git_branch('master') is not None: - base.branch = 'master' + base.ref = 'master' else: ex = RecipeException("Cannot find default 'main' or 'master' branch") ex.hint = "Please specify the base branch using the '--base-branch' option." @@ -90,12 +90,12 @@ def _create_target_recipe(dbt_project_path=None, options: dict = None) -> Recipe target = RecipeModel() if tool().git_branch() is not None: - if options.get('target_branch') is not None: - if tool().git_branch(options.get('target_branch')) is not None: - target.branch = options.get('target_branch') + if options.get('target_ref') is not None: + if tool().git_branch(options.get('target_ref')) is not None: + target.ref = options.get('target_ref') else: - ex = RecipeException(f"Cannot find specified base branch: {options.get('target_branch')}") - ex.hint = f"Please check if the specified branch name '{options.get('target_branch')}' is correct." + ex = RecipeException(f"Cannot find specified base branch: {options.get('target_ref')}") + ex.hint = f"Please check if the specified branch name '{options.get('target_ref')}' is correct." raise ex dbt_project = _read_dbt_project_file(dbt_project_path) diff --git a/piperider_cli/recipes/github_action.py b/piperider_cli/recipes/github_action.py index 370db9ea4..8fc4ee5aa 100644 --- a/piperider_cli/recipes/github_action.py +++ b/piperider_cli/recipes/github_action.py @@ -42,12 +42,12 @@ def prepare_for_action(recipe_name: str = None): if recipe_path: print(f'[Prepare] check recipe from file: {recipe_path}') cfg = RecipeConfiguration.load(recipe_path) - if cfg.base.branch: - print(f'[Prepare] switch to base branch: {cfg.base.branch}') - git_switch_to(cfg.base.branch) - if cfg.target.branch: - print(f'[Prepare] switch to target branch: {cfg.target.branch}') - git_switch_to(cfg.target.branch) + if cfg.base.ref: + print(f'[Prepare] switch to base branch: {cfg.base.ref}') + git_switch_to(cfg.base.ref) + if cfg.target.ref: + print(f'[Prepare] switch to target branch: {cfg.target.ref}') + git_switch_to(cfg.target.ref) head_ref = os.environ.get('GITHUB_HEAD_REF') print(f'[Prepare] switch to GITHUB_HEAD_REF: {head_ref}') From f14cfa15bd87c67566fb226e5ed057dfa644c279 Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Thu, 14 Sep 2023 16:27:46 +0800 Subject: [PATCH 4/9] Fix some wording problem Signed-off-by: Wei-Chun, Chang --- piperider_cli/recipes/default_recipe_generator.py | 8 ++++---- tests/test_recipe.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/piperider_cli/recipes/default_recipe_generator.py b/piperider_cli/recipes/default_recipe_generator.py index c516aa599..9af014549 100644 --- a/piperider_cli/recipes/default_recipe_generator.py +++ b/piperider_cli/recipes/default_recipe_generator.py @@ -62,8 +62,8 @@ def _create_base_recipe(dbt_project_path=None, options: dict = None) -> RecipeMo if tool().git_branch(options.get('base_ref')) is not None: base.ref = options.get('base_ref') else: - ex = RecipeException(f"Cannot find specified base branch: {options.get('base_ref')}") - ex.hint = f"Please check if the specified branch name '{options.get('base_ref')}' is correct." + ex = RecipeException(f"Cannot find specified base ref: {options.get('base_ref')}") + ex.hint = f"Please check if the specified ref name '{options.get('base_ref')}' is correct." raise ex else: if tool().git_branch('main') is not None: @@ -94,8 +94,8 @@ def _create_target_recipe(dbt_project_path=None, options: dict = None) -> Recipe if tool().git_branch(options.get('target_ref')) is not None: target.ref = options.get('target_ref') else: - ex = RecipeException(f"Cannot find specified base branch: {options.get('target_ref')}") - ex.hint = f"Please check if the specified branch name '{options.get('target_ref')}' is correct." + ex = RecipeException(f"Cannot find specified base ref: {options.get('target_ref')}") + ex.hint = f"Please check if the specified ref name '{options.get('target_ref')}' is correct." raise ex dbt_project = _read_dbt_project_file(dbt_project_path) diff --git a/tests/test_recipe.py b/tests/test_recipe.py index 0e5b8508d..3e38dc330 100644 --- a/tests/test_recipe.py +++ b/tests/test_recipe.py @@ -28,7 +28,7 @@ def test_update_select_with_cli_option(self): self.assertEqual((), select) def test_prepare_piperider_cmd(self): - options = dict(base_branch=None, skip_datasource_connection=False) + options = dict(base_ref=None, target_ref=None, skip_datasource_connection=False) # base dbt_field = _prepare_dbt_cmds(options) From 127a1c90ce5f272f15e46fcdad290f47c299ce6b Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Thu, 14 Sep 2023 17:42:43 +0800 Subject: [PATCH 5/9] Refactor and add ut Signed-off-by: Wei-Chun, Chang --- piperider_cli/cli.py | 31 ++++++------------- .../cli_utils/compare_with_recipe.py | 21 +++++++++++++ tests/test_recipe.py | 28 +++++++++++++++++ 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/piperider_cli/cli.py b/piperider_cli/cli.py index b7a5bc91a..96230daa0 100644 --- a/piperider_cli/cli.py +++ b/piperider_cli/cli.py @@ -430,7 +430,7 @@ def cloud_compare_reports(**kwargs): @cli.command(name='compare', short_help='Compare the change for the current branch.', cls=TrackCommand) -@click.argument('commits', nargs=-1, type=click.STRING) +@click.argument('refs', nargs=-1, type=click.STRING) @click.option('--recipe', default=None, type=click.STRING, help='Select a different recipe.') @click.option('--upload', default=False, is_flag=True, help='Upload the report to PipeRider Cloud.') @click.option('--share', default=False, is_flag=True, help='Enable public share of the report to PipeRider Cloud.') @@ -451,32 +451,21 @@ def cloud_compare_reports(**kwargs): ]) @add_options(dbt_related_options) @add_options(debug_option) -def compare_with_recipe(commits, **kwargs): +def compare_with_recipe(refs, **kwargs): """ Generate the comparison report for your branch. """ - console = Console() - if len(commits) > 1: - console.print('[bold red]Error:[/bold red] Commit format is not supported') - sys.exit(1) - elif len(commits) == 1: - commits = commits[0] - if '...' in commits: - kwargs['base_ref'] = commits.split('...')[0] - kwargs['target_ref'] = commits.split('...')[1] - elif '..' in commits: - console.print('[bold red]Error:[/bold red] Two-dot diff comparisons are not supported') - sys.exit(1) - else: - kwargs['base_ref'] = commits - - if kwargs.get('base_branch') and kwargs.get('base_ref'): - console.print("[bold red]Error:[/bold red] '--base-branch' option and commit argument cannot be used together") + from piperider_cli.cli_utils.compare_with_recipe import assign_compare_ref, compare_with_recipe as cmd + options = kwargs + err, msg = assign_compare_ref(refs, options) + + if err != 0: + console = Console() + console.print(msg) sys.exit(1) - from piperider_cli.cli_utils.compare_with_recipe import compare_with_recipe as cmd - return cmd(**kwargs) + return cmd(**options) @cloud.command(short_help='Signup to PipeRider Cloud.', cls=TrackCommand) diff --git a/piperider_cli/cli_utils/compare_with_recipe.py b/piperider_cli/cli_utils/compare_with_recipe.py index f08365498..570d40b1b 100644 --- a/piperider_cli/cli_utils/compare_with_recipe.py +++ b/piperider_cli/cli_utils/compare_with_recipe.py @@ -1,3 +1,24 @@ +def assign_compare_ref(refs: tuple, options: dict): + if len(refs) > 1: + return -1, '[bold red]Error:[/bold red] Commit format is not supported' + elif len(refs) == 1: + refs = refs[0] + if '...' in refs: + options['base_ref'] = refs.split('...')[0] + options['target_ref'] = refs.split('...')[1] + if options['base_ref'] == '' or options['target_ref'] == '': + return -1, '[bold red]Error:[/bold red] Commit format is not supported' + elif '..' in refs: + return -1, '[bold red]Error:[/bold red] Two-dot diff comparisons are not supported' + else: + options['base_ref'] = refs + + if options.get('base_branch') and options.get('base_ref'): + return -1, "[bold red]Error:[/bold red] '--base-branch' option and '[REF]' argument cannot be used together" + + return 0, None + + def compare_with_recipe(**kwargs): """ Generate the comparison report for your branch. diff --git a/tests/test_recipe.py b/tests/test_recipe.py index 3e38dc330..3f71345fc 100644 --- a/tests/test_recipe.py +++ b/tests/test_recipe.py @@ -1,5 +1,6 @@ from unittest import TestCase +from piperider_cli.cli_utils.compare_with_recipe import assign_compare_ref from piperider_cli.recipes import update_select_with_cli_option from piperider_cli.recipes.default_recipe_generator import _prepare_dbt_cmds, _prepare_piperider_cmd @@ -77,3 +78,30 @@ def test_prepare_piperider_cmd(self): dbt_field.commands) self.assertEqual(['piperider run --skip-datasource'], piperider_field.commands) + def test_assign_compare_ref(self): + options = dict(base_branch=None) + + refs = ('master', 'develop') + err, _ = assign_compare_ref(refs, options) + self.assertEqual(-1, err) + + refs = ('master..develop',) + err, _ = assign_compare_ref(refs, options) + self.assertEqual(-1, err) + + refs = ('master...',) + err, _ = assign_compare_ref(refs, options) + self.assertEqual(-1, err) + + refs = ('master...develop',) + err, _ = assign_compare_ref(refs, options) + self.assertEqual(0, err) + + refs = ('master',) + err, _ = assign_compare_ref(refs, options) + self.assertEqual(0, err) + + options['base_branch'] = 'master' + refs = ('develop',) + err, _ = assign_compare_ref(refs, options) + self.assertEqual(-1, err) From d5a1b16c4c68c9440c6629a0a67db2e54899c8c8 Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Fri, 15 Sep 2023 10:44:53 +0800 Subject: [PATCH 6/9] Remove ref information if it is not branch Signed-off-by: Wei-Chun, Chang --- piperider_cli/recipes/__init__.py | 3 ++- piperider_cli/recipes/utils.py | 9 +++++++++ piperider_cli/runner.py | 3 ++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/piperider_cli/recipes/__init__.py b/piperider_cli/recipes/__init__.py index 430319163..43f09f9bf 100644 --- a/piperider_cli/recipes/__init__.py +++ b/piperider_cli/recipes/__init__.py @@ -437,7 +437,8 @@ def execute_recipe_archive(cfg: RecipeConfiguration, recipe_type='base', debug=F model.tmp_dir_path = tool().git_archive(branch_or_commit) # NOTE: Passing git branch information through environment variables, # as the archived folder does not have a .git folder. - model.piperider.environments['PIPERIDER_GIT_BRANCH'] = model.ref + if tool().git_is_ref_branch(model.ref): + model.piperider.environments['PIPERIDER_GIT_BRANCH'] = model.ref model.piperider.environments['PIPERIDER_GIT_SHA'] = branch_or_commit console.print() diff --git a/piperider_cli/recipes/utils.py b/piperider_cli/recipes/utils.py index e20424385..896f6d9b0 100644 --- a/piperider_cli/recipes/utils.py +++ b/piperider_cli/recipes/utils.py @@ -106,6 +106,15 @@ def git_rev_parse(self, obj_name: str = 'HEAD'): return outs + def git_is_ref_branch(self, obj_name: str) -> bool: + outs, errs, exit_code = self.dryrun_ignored_execute_command( + f"git show-ref --verify refs/heads/{obj_name}" + ) + if exit_code != 0: + return False + + return True + def git_checkout_to(self, commit_or_branch): outs, errs, exit_code = self.execute_command_in_silent( f"git checkout {commit_or_branch}" diff --git a/piperider_cli/runner.py b/piperider_cli/runner.py index 44cc68e4c..4c0855063 100644 --- a/piperider_cli/runner.py +++ b/piperider_cli/runner.py @@ -605,7 +605,8 @@ def get_git_branch(): # NOTE: Provide git branch information directly for the archived dbt project without .git folder git_branch = os.environ.get('PIPERIDER_GIT_BRANCH', None) git_sha = os.environ.get('PIPERIDER_GIT_SHA', None) - if git_branch is not None and git_sha is not None: + # there's no git branch information when comparing to tag or commit + if git_branch is not None or git_sha is not None: return git_branch, git_sha def _exec(command_line: str): From 11a738bbc728a181ec4feb04bacdd8ad7a0d1639 Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Fri, 15 Sep 2023 15:31:25 +0800 Subject: [PATCH 7/9] Get branch information for HEAD reference Signed-off-by: Wei-Chun, Chang --- piperider_cli/recipes/__init__.py | 2 ++ piperider_cli/recipes/utils.py | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/piperider_cli/recipes/__init__.py b/piperider_cli/recipes/__init__.py index 43f09f9bf..48334803b 100644 --- a/piperider_cli/recipes/__init__.py +++ b/piperider_cli/recipes/__init__.py @@ -439,6 +439,8 @@ def execute_recipe_archive(cfg: RecipeConfiguration, recipe_type='base', debug=F # as the archived folder does not have a .git folder. if tool().git_is_ref_branch(model.ref): model.piperider.environments['PIPERIDER_GIT_BRANCH'] = model.ref + elif model.ref == 'HEAD': + model.piperider.environments['PIPERIDER_GIT_BRANCH'] = tool().git_current_branch() model.piperider.environments['PIPERIDER_GIT_SHA'] = branch_or_commit console.print() diff --git a/piperider_cli/recipes/utils.py b/piperider_cli/recipes/utils.py index 896f6d9b0..0fa7f4cf2 100644 --- a/piperider_cli/recipes/utils.py +++ b/piperider_cli/recipes/utils.py @@ -115,6 +115,15 @@ def git_is_ref_branch(self, obj_name: str) -> bool: return True + def git_current_branch(self): + outs, errs, exit_code = self.dryrun_ignored_execute_command( + f"git branch --show-current" + ) + if exit_code != 0: + return None + + return outs + def git_checkout_to(self, commit_or_branch): outs, errs, exit_code = self.execute_command_in_silent( f"git checkout {commit_or_branch}" From 95bcdbcc730f561f6257bfdc2b9cf56dbfca9ee2 Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Fri, 15 Sep 2023 16:31:34 +0800 Subject: [PATCH 8/9] Change compare argument setting Signed-off-by: Wei-Chun, Chang --- piperider_cli/cli.py | 16 ++---- .../cli_utils/compare_with_recipe.py | 56 ++++++++++++------- piperider_cli/recipes/utils.py | 2 +- tests/test_recipe.py | 54 +++++++++--------- 4 files changed, 66 insertions(+), 62 deletions(-) diff --git a/piperider_cli/cli.py b/piperider_cli/cli.py index 96230daa0..a54307f1b 100644 --- a/piperider_cli/cli.py +++ b/piperider_cli/cli.py @@ -430,7 +430,7 @@ def cloud_compare_reports(**kwargs): @cli.command(name='compare', short_help='Compare the change for the current branch.', cls=TrackCommand) -@click.argument('refs', nargs=-1, type=click.STRING) +@click.argument('ref-diff', required=False, type=click.STRING) @click.option('--recipe', default=None, type=click.STRING, help='Select a different recipe.') @click.option('--upload', default=False, is_flag=True, help='Upload the report to PipeRider Cloud.') @click.option('--share', default=False, is_flag=True, help='Enable public share of the report to PipeRider Cloud.') @@ -451,21 +451,13 @@ def cloud_compare_reports(**kwargs): ]) @add_options(dbt_related_options) @add_options(debug_option) -def compare_with_recipe(refs, **kwargs): +def compare_with_recipe(ref_diff, **kwargs): """ Generate the comparison report for your branch. """ - from piperider_cli.cli_utils.compare_with_recipe import assign_compare_ref, compare_with_recipe as cmd - options = kwargs - err, msg = assign_compare_ref(refs, options) - - if err != 0: - console = Console() - console.print(msg) - sys.exit(1) - - return cmd(**options) + from piperider_cli.cli_utils.compare_with_recipe import compare_with_recipe as cmd + return cmd(ref_diff, **kwargs) @cloud.command(short_help='Signup to PipeRider Cloud.', cls=TrackCommand) diff --git a/piperider_cli/cli_utils/compare_with_recipe.py b/piperider_cli/cli_utils/compare_with_recipe.py index 570d40b1b..2bda17736 100644 --- a/piperider_cli/cli_utils/compare_with_recipe.py +++ b/piperider_cli/cli_utils/compare_with_recipe.py @@ -1,25 +1,29 @@ -def assign_compare_ref(refs: tuple, options: dict): - if len(refs) > 1: - return -1, '[bold red]Error:[/bold red] Commit format is not supported' - elif len(refs) == 1: - refs = refs[0] - if '...' in refs: - options['base_ref'] = refs.split('...')[0] - options['target_ref'] = refs.split('...')[1] - if options['base_ref'] == '' or options['target_ref'] == '': - return -1, '[bold red]Error:[/bold red] Commit format is not supported' - elif '..' in refs: - return -1, '[bold red]Error:[/bold red] Two-dot diff comparisons are not supported' - else: - options['base_ref'] = refs +from rich.console import Console + + +def parse_compare_ref(refs: str): + console = Console() + + if refs is None: + return None, None - if options.get('base_branch') and options.get('base_ref'): - return -1, "[bold red]Error:[/bold red] '--base-branch' option and '[REF]' argument cannot be used together" + if '...' in refs: + base_ref = refs.split('...')[0] + target_ref = refs.split('...')[1] + if base_ref == '' or target_ref == '': + console.print('[bold red]Error:[/bold red] Commit format is not supported') + return None, None + elif '..' in refs: + console.print('[bold red]Error:[/bold red] Two-dot diff comparisons are not supported') + return None, None + else: + base_ref = refs + target_ref = None - return 0, None + return base_ref, target_ref -def compare_with_recipe(**kwargs): +def compare_with_recipe(ref_diff, **kwargs): """ Generate the comparison report for your branch. """ @@ -31,6 +35,8 @@ def compare_with_recipe(**kwargs): from piperider_cli.initializer import Initializer from piperider_cli.recipes import RecipeConfiguration, configure_recipe_execution_flags, is_recipe_dry_run + console = Console() + recipe = kwargs.get('recipe') summary_file = kwargs.get('summary_file') force_upload = kwargs.get('upload') @@ -40,11 +46,19 @@ def compare_with_recipe(**kwargs): debug = kwargs.get('debug', False) select = kwargs.get('select') modified = kwargs.get('modified') - - base_ref = kwargs.get('base_branch') if kwargs.get('base_branch') else kwargs.get('base_ref') - target_ref = kwargs.get('target_ref') skip_datasource_connection = kwargs.get('skip_datasource') + base_ref, target_ref = parse_compare_ref(ref_diff) + if ref_diff is not None and base_ref is None: + return -1 + + if base_ref is not None and kwargs.get('base_branch') is not None: + console.print("[bold red]Error:[/bold red] " + "'--base-branch' option and '[REF-DIFF]' argument cannot be used together") + return -1 + elif base_ref is None: + base_ref = kwargs.get('base_branch') + # reconfigure recipe global flags configure_recipe_execution_flags(dry_run=kwargs.get('dry_run'), interactive=kwargs.get('interactive')) diff --git a/piperider_cli/recipes/utils.py b/piperider_cli/recipes/utils.py index 0fa7f4cf2..633ad5b4e 100644 --- a/piperider_cli/recipes/utils.py +++ b/piperider_cli/recipes/utils.py @@ -117,7 +117,7 @@ def git_is_ref_branch(self, obj_name: str) -> bool: def git_current_branch(self): outs, errs, exit_code = self.dryrun_ignored_execute_command( - f"git branch --show-current" + "git branch --show-current" ) if exit_code != 0: return None diff --git a/tests/test_recipe.py b/tests/test_recipe.py index 3f71345fc..0ee507428 100644 --- a/tests/test_recipe.py +++ b/tests/test_recipe.py @@ -1,6 +1,6 @@ from unittest import TestCase -from piperider_cli.cli_utils.compare_with_recipe import assign_compare_ref +from piperider_cli.cli_utils.compare_with_recipe import parse_compare_ref from piperider_cli.recipes import update_select_with_cli_option from piperider_cli.recipes.default_recipe_generator import _prepare_dbt_cmds, _prepare_piperider_cmd @@ -78,30 +78,28 @@ def test_prepare_piperider_cmd(self): dbt_field.commands) self.assertEqual(['piperider run --skip-datasource'], piperider_field.commands) - def test_assign_compare_ref(self): - options = dict(base_branch=None) - - refs = ('master', 'develop') - err, _ = assign_compare_ref(refs, options) - self.assertEqual(-1, err) - - refs = ('master..develop',) - err, _ = assign_compare_ref(refs, options) - self.assertEqual(-1, err) - - refs = ('master...',) - err, _ = assign_compare_ref(refs, options) - self.assertEqual(-1, err) - - refs = ('master...develop',) - err, _ = assign_compare_ref(refs, options) - self.assertEqual(0, err) - - refs = ('master',) - err, _ = assign_compare_ref(refs, options) - self.assertEqual(0, err) - - options['base_branch'] = 'master' - refs = ('develop',) - err, _ = assign_compare_ref(refs, options) - self.assertEqual(-1, err) + def test_parse_compare_ref(self): + refs = None + base_ref, target_ref = parse_compare_ref(refs) + self.assertEqual(None, base_ref) + self.assertEqual(None, target_ref) + + refs = 'master..develop' + base_ref, target_ref = parse_compare_ref(refs) + self.assertEqual(None, base_ref) + self.assertEqual(None, target_ref) + + refs = 'master...' + err, _ = parse_compare_ref(refs) + self.assertEqual(None, base_ref) + self.assertEqual(None, target_ref) + + refs = 'master...develop' + base_ref, target_ref = parse_compare_ref(refs) + self.assertEqual('master', base_ref) + self.assertEqual('develop', target_ref) + + refs = 'master' + base_ref, target_ref = parse_compare_ref(refs) + self.assertEqual('master', base_ref) + self.assertEqual(None, target_ref) From 1d58ce54af3772259cfee30854e073ac2dbcd30e Mon Sep 17 00:00:00 2001 From: "Wei-Chun, Chang" Date: Fri, 15 Sep 2023 16:52:14 +0800 Subject: [PATCH 9/9] Add helper and fix wording Signed-off-by: Wei-Chun, Chang --- piperider_cli/cli.py | 26 ++++++++++++++++--- .../cli_utils/compare_with_recipe.py | 25 +++++++++--------- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/piperider_cli/cli.py b/piperider_cli/cli.py index a54307f1b..c3d41eb1a 100644 --- a/piperider_cli/cli.py +++ b/piperider_cli/cli.py @@ -430,7 +430,7 @@ def cloud_compare_reports(**kwargs): @cli.command(name='compare', short_help='Compare the change for the current branch.', cls=TrackCommand) -@click.argument('ref-diff', required=False, type=click.STRING) +@click.argument('ref', required=False, type=click.STRING) @click.option('--recipe', default=None, type=click.STRING, help='Select a different recipe.') @click.option('--upload', default=False, is_flag=True, help='Upload the report to PipeRider Cloud.') @click.option('--share', default=False, is_flag=True, help='Enable public share of the report to PipeRider Cloud.') @@ -451,13 +451,33 @@ def cloud_compare_reports(**kwargs): ]) @add_options(dbt_related_options) @add_options(debug_option) -def compare_with_recipe(ref_diff, **kwargs): +def compare_with_recipe(ref, **kwargs): """ Generate the comparison report for your branch. + + \b + # compare with main/master branch + piperider compare + + \b + # compare with specific branch + piperider compare --base-branch + + \b + # compare with any reference + piperider compare + + \b + # compare with two references + piperider compare ... + + \b + Note: can be reference that git understands. e.g., branch, tag, commit, etc. + """ from piperider_cli.cli_utils.compare_with_recipe import compare_with_recipe as cmd - return cmd(ref_diff, **kwargs) + return cmd(ref, **kwargs) @cloud.command(short_help='Signup to PipeRider Cloud.', cls=TrackCommand) diff --git a/piperider_cli/cli_utils/compare_with_recipe.py b/piperider_cli/cli_utils/compare_with_recipe.py index 2bda17736..90ebb0f3b 100644 --- a/piperider_cli/cli_utils/compare_with_recipe.py +++ b/piperider_cli/cli_utils/compare_with_recipe.py @@ -1,29 +1,30 @@ from rich.console import Console -def parse_compare_ref(refs: str): +def parse_compare_ref(ref: str): console = Console() - if refs is None: + if ref is None: return None, None - if '...' in refs: - base_ref = refs.split('...')[0] - target_ref = refs.split('...')[1] + if '...' in ref: + base_ref = ref.split('...')[0] + target_ref = ref.split('...')[1] if base_ref == '' or target_ref == '': - console.print('[bold red]Error:[/bold red] Commit format is not supported') + console.print('[bold red]Error:[/bold red] ' + 'Please either provide a single git reference or a 3-dot diff comparison form.') return None, None - elif '..' in refs: + elif '..' in ref: console.print('[bold red]Error:[/bold red] Two-dot diff comparisons are not supported') return None, None else: - base_ref = refs + base_ref = ref target_ref = None return base_ref, target_ref -def compare_with_recipe(ref_diff, **kwargs): +def compare_with_recipe(ref, **kwargs): """ Generate the comparison report for your branch. """ @@ -48,13 +49,13 @@ def compare_with_recipe(ref_diff, **kwargs): modified = kwargs.get('modified') skip_datasource_connection = kwargs.get('skip_datasource') - base_ref, target_ref = parse_compare_ref(ref_diff) - if ref_diff is not None and base_ref is None: + base_ref, target_ref = parse_compare_ref(ref) + if ref is not None and base_ref is None: return -1 if base_ref is not None and kwargs.get('base_branch') is not None: console.print("[bold red]Error:[/bold red] " - "'--base-branch' option and '[REF-DIFF]' argument cannot be used together") + "'--base-branch' option and '[REF]' argument cannot be used together") return -1 elif base_ref is None: base_ref = kwargs.get('base_branch')