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

[CT-569] [Feature] Allow profile_template to prompt and name a target #5179

Closed
1 task done
alexrosenfeld10 opened this issue Apr 27, 2022 · 7 comments · Fixed by #5184
Closed
1 task done

[CT-569] [Feature] Allow profile_template to prompt and name a target #5179

alexrosenfeld10 opened this issue Apr 27, 2022 · 7 comments · Fixed by #5184
Labels
enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors! init Issues related to initializing the dbt starter project

Comments

@alexrosenfeld10
Copy link
Contributor

Is this your first time opening an issue?

Describe the Feature

I'm not 100% sure if this is or isn't yet an option, but I didn't find it anywhere online in searching.

It'd be nice if profile_template.yml allowed to set the target name, like so:

prompts:
  target:
    type: string
    hint: the target name

or,

fixed:
    target: somethig_here

Describe alternatives you've considered

No response

Who will this benefit?

No response

Are you interested in contributing this feature?

No response

Anything else?

No response

@alexrosenfeld10 alexrosenfeld10 added enhancement New feature or request triage labels Apr 27, 2022
@github-actions github-actions bot changed the title [Feature] Allow profile_template to prompt and name a target [CT-569] [Feature] Allow profile_template to prompt and name a target Apr 27, 2022
@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 28, 2022

@alexrosenfeld10 Right on! I think this would be a self-contained change, and probably a simple one:

prompts = profile_template.get("prompts", {})
target = self.generate_target_from_input(prompts, initial_target)
profile = {"outputs": {"dev": target}, "target": "dev"}

It might look like inspecting the output of generate_target_from_input, and if target is among the prompts provided, pulling it up to be the higher-level key (instead of the default dev).

Is this something you might be interested in contributing?

There's just one tricky piece of this, which will be writing an automated test. We need to refactor and convert the tests for init behavior (#5180) to account for some changes to mypy (#5171). As long as there's a test case recorded in there, to be converted along with the rest of 040_init_tests, that should be fine.

@jtcohen6 jtcohen6 added good_first_issue Straightforward + self-contained changes, good for new contributors! init Issues related to initializing the dbt starter project and removed triage labels Apr 28, 2022
@alexrosenfeld10
Copy link
Contributor Author

@jtcohen6 Sure, happy to contribute. I agree, that's definitely an easy way to solve this, however it does allow for a user to set syntactically invalid targets in their init flow. Is that something we want to care about here? In thinking about this I noticed that the profile template also allows other silly things, such as:

prompts:
  threads:
    type: string
  i_am_not_a_real_thing:
    type: string
    hint: blargh
  user:
    type: bool
    hint: your snowflake username... which probably isn't a bool

If we want, it'd be pretty easy to add some simple "am I valid yaml" regexing to the input target.

Entirely possible we just don't care / I'm overthinking this. Here's a PR #5184 with the naive approach coded out

@alexrosenfeld10
Copy link
Contributor Author

Relevant docs update here: dbt-labs/docs.getdbt.com#1388

wasn't sure what to do about the versioning, just going to leave it at that for now and open an issue for the docs side of things

@jtcohen6
Copy link
Contributor

@alexrosenfeld10 Appreciate the thought you've given this! The folks creating profile_template.yml tend to be:

  • Adapter plugin maintainers
  • Maintainers of mature projects at larger organizations

So, definitely not most users, and definitely people with some prior dbt experience. I think the naive solution is probably okay for now. But we could definitely do more here to help over the long run.

I'm also thinking about the fact that a lot of the information in profile_template.yml (e.g. dbt-snowflake's) is duplicative with the Credentials dataclass defined in each adapter plugin (e.g. dbt-snowflake's). So: some way to reuse and enforce names + types, while also allowing an override of defaults.

@alexrosenfeld10
Copy link
Contributor Author

I agree that's the folks creating the template.. but folks inputting options in response to dbt init are usually the newbies I'd be worried about 😄 but sure, I generally agree that people can choose to hold the knife by the handle on this one

@alexrosenfeld10
Copy link
Contributor Author

@jtcohen6 I think this is worth merging, but upon further thought it is kind of a mediocre way to cover my use case.

Expanding on this a bit more - is there any reason dbt doesn't allow arbitrary properties to be set in the profile? I'm after a turn-key experience where dbt init handles 100% of the setup local developers need... To do this, I am overriding the generate_database_name and generate_schema_name macros. I need two pieces of metadata:

  1. their developer schema name (which usually matches username, but not always), because they each get their own schema when running locally
  2. their environment, because I use snowflake and have logical environments in the same physical instance.

Ideally, I'd love to do something like this:

my_project:
  outputs:
    local_arosenfeld:
      account: my.account
...
      snowflake_env: qa
      snowflake_schema: alex_dev

The resulting database and schema for such a configuration would yield db_name_qa.local_alex_dev

The only way I found to do such a thing is by using target.name in the macro overrides, hence this issue + PR, because while you're free to set any properties in the profiles.yml file, anything no expected gets stripped out and isn't found on {{ target }} in the macro.

Because I can't set custom properties, I'm using the target name. However, that's far from ideal, especially when you need to work in two pieces of metadata (I'm getting around that with a double underscore + .split('__')). Overall, feels a bit kludgy.

@alexrosenfeld10
Copy link
Contributor Author

alexrosenfeld10 commented Apr 28, 2022

The more I think about it the more I realize that env vars are just better for this. That just means I need an extra step for folks to get set up running locally. Not the end of the world, but as someone buildin dbt project setups for many upstream teams to consume, all of which are novices in the dbt space, it'd be nice to have dbt init and the profile configuration itself allow for overriding env vars / adding custom properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors! init Issues related to initializing the dbt starter project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants