Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Add DbtSnapshotOperator, test and update Readme #13

Closed
wants to merge 2 commits into from

Conversation

faouzelfassi
Copy link

@faouzelfassi faouzelfassi commented Jul 29, 2020

Add DbtSnapshotOperator: that calls dbt snapshot #12

@faouzelfassi faouzelfassi changed the title Add DbtSnapshotOperator, test and update Readme #12 Add DbtSnapshotOperator, test and update Readme Jul 29, 2020
Copy link
Contributor

@rliddler rliddler left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @faouzelfassi !

I've noticed there seem to be a lot of formatting changes (I guess automatically added by your linter?) if you could remove those so it's a PR of just the required changes that would be great!

I believe we have a line limit of 110 specified in our flake8 for this project.

@prratek
Copy link

prratek commented Aug 17, 2020

The dbt snapshot CLI command supports a selector that's used like dbt snapshot --select some_model. To support passing in a selector, would we need an additional parameter for the DbtCliHook?

@andrewrjones
Copy link
Contributor

I wasn't able to fix the formatting on this PR so have created a new PR from @faouzelfassi's commits that is ready for review: #22. Closing this one.

@prratek Not sure how best to support the additional options, but would probably suggest trying to change the signature of run_cli to take optional arguments. Created an issue for that here: #23

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants