Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Remove stateful selectors from palm run #73

Conversation

jakeberesford-palmetto
Copy link
Collaborator

Pull request checklist

Before submitting your PR, please review the following checklist:

  • Consider adding a unit test if your PR resolves an issue.
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Breaking changes

  • Check if this pull request introduces a breaking change

What does this implement/fix? Explain your changes.

palm run was applying stateful selectors, while dbt was not using a stateful run. This makes it difficult to run the entire project and introduces an unexpected state where no models are executed.

Any other comments?

Also fixed up some whitespace in the command (--fail-fast changes)

Where has this been tested?

Operating System: MacOS

Platform: Python3.9

…, as dbt run does.

- Only add stateful selection on deferred runs
- Fix whitespace around `full-refresh logic and make the condition a one-liner
@@ -128,6 +123,7 @@ def build_run_command(
if not no_fail_fast:
cmd.append("--fail-fast")
if defer:
cmd.extend(["--select", "state:new", "state:modified+"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be included every time the defer flag is used. Defer can be used with any model selections.

I think possibly the right condition would be if not targets and defer: for when to pass in state:new/modified. When no targets and not running with defer, default to no selection (aka run the full project or the default dbt selector).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this makes sense. I'll make this change.

We want to support deferred runs with selectors.
Copy link
Collaborator

@mariahjrogers mariahjrogers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for fixing this!

@jakeberesford-palmetto jakeberesford-palmetto merged commit 1e80d50 into palmetto:develop Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants