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

feat: add dbt_picker_upstream and dbt_picker_downstream for relative model selection #35

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cmpadden
Copy link
Contributor

@cmpadden cmpadden commented Nov 9, 2024

Introduces dbt_picker_upstream and dbt_picker_downstream Telescope pickers.

Makes use of the --select option of the dbt ls command. By passing in the relative file path, to the dbt project, of the current buffer, we can determine the upstream and downstream models with either a prefix or suffix +.

Definitely open to feedback on the implementation, I think it might be possible for us to implement a generic solution where the user can provide the --select syntax themselves, and _picker might be able to be cleaned up a bit more.

@cmpadden
Copy link
Contributor Author

cmpadden commented Nov 9, 2024

Closes #34

lua/dbtpal/telescope.lua Outdated Show resolved Hide resolved
@cmpadden
Copy link
Contributor Author

Just had the thought that we could prompt the user for a "select" argument, possibly with the current buffer pre-populated.

@Doekeb
Copy link

Doekeb commented Nov 13, 2024

Nice! The basics seem to be working for me. One note is that it seems to also be picking up data tests as well as models. I'd prefer only models by default.

Unfortunately, these pickers are the main things I want to use this extension for and they are wayyyy to slow to be useful at the moment (even the existing all model picker).

@cmpadden
Copy link
Contributor Author

cmpadden commented Nov 15, 2024

Nice! The basics seem to be working for me. One note is that it seems to also be picking up data tests as well as models. I'd prefer only models by default.

Unfortunately, these pickers are the main things I want to use this extension for and they are wayyyy to slow to be useful at the moment (even the existing all model picker).

Good catch, I fixed it in this commit: 3f7594f. There was an issue with how I was using vim.tbl_extend which was resolved by using vim.list_extend for the additional args.

Totally agree on performance. My first thought on caching was that we would store the upstream / downstream models when the picker is used, and then invalidate the cache if a model in the cache were modified. But this wouldn't account for creating a new model. Caching might not be a tricky one... 🤔

@Doekeb
Copy link

Doekeb commented Nov 17, 2024

Have you tried passing --partial-parse to the dbt command?
https://docs.getdbt.com/reference/global-configs/parsing#partial-parsing

@cmpadden
Copy link
Contributor Author

Have you tried passing --partial-parse to the dbt command? https://docs.getdbt.com/reference/global-configs/parsing#partial-parsing

This project is quite small, so it may not be a good representation of performance, but the time is comparable. Note the log statement that there was no saved manifest so it's a full parse on the first run (1.560s), and then partial on the second (1.336s).

$ time dbt ls --partial-parse --select=my_first_model+
21:16:25  Running with dbt=1.8.8
21:16:25  Registered adapter: duckdb=1.9.0
21:16:25  Unable to do partial parsing because saved manifest not found. Starting full parse.    <-----
21:16:26  Found 2 models, 4 data tests, 416 macros
21:16:26  The selection criterion 'my_first_model+' does not match any enabled nodes
21:16:26  No nodes selected!

real    0m1.560s
user    0m1.385s
sys     0m0.124s

$ time dbt ls --partial-parse --select=my_first_model+
21:18:58  Running with dbt=1.8.8
21:18:58  Registered adapter: duckdb=1.9.0
21:18:58  Found 2 models, 4 data tests, 416 macros
21:18:58  The selection criterion 'my_first_model+' does not match any enabled nodes
21:18:58  No nodes selected!

real    0m1.336s
user    0m0.918s
sys     0m0.146s

When I'm on my other computer I can test a real-world project; would be curious what you get too!

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