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

Docker instructions #34

Merged
merged 2 commits into from
Oct 5, 2022
Merged

Docker instructions #34

merged 2 commits into from
Oct 5, 2022

Conversation

dbeatty10
Copy link
Collaborator

@dbeatty10 dbeatty10 commented Oct 1, 2022

Resolves #33

Overview

  1. profiles.yml via DBT_PROFILES_DIR no longer needed
  2. Updates to documentation for Docker

profiles.yml via DBT_PROFILES_DIR

As of dbt Core 1.3, setting the DBT_PROFILES_DIR environment variable is no longer necessary:

ENV DBT_PROFILES_DIR=/workspaces/jaffle_shop_duckdb

Documentation

Also, made some slight updates to the instructions for running Docker with VSCode locally.

Validation

These changes can be validated by:

  1. Launching Docker locally in VSCode (or in GitHub Codespaces)
  2. Run dbt debug and noting that it is looking for profiles in the intended directory (and connecting successfully)
  3. Run dbt build

@dbeatty10 dbeatty10 marked this pull request as ready for review October 1, 2022 16:26
@dbeatty10 dbeatty10 requested a review from dataders October 1, 2022 16:27
@dataders
Copy link
Contributor

dataders commented Oct 4, 2022

@dbeatty10 can you link me to the PR for the change in core? and is it ok that we don't currently call out that this example only works for dbt-core>=1.3.0

@dbeatty10
Copy link
Collaborator Author

can you link me to the PR for the change in core?

Here is the PR that added this to Core 1.3.

is it ok that we don't currently call out that this example only works for dbt-core>=1.3.0?

On one hand

It's probably okay since this is explicitly intended as a demo project rather than a project template. Following the readme instructions will automatically get dbt-core 1.3 because of this line.

On the other

If we don't mind the extra text, we could also call out this minimum version in the README somewhere. But the README already feels way too long to me currently -- I'd love to reduce the amount of text dramatically to something more like this.

Optionally, we could further support a minimum version of 1.3.0 via require-dbt-version
here.

@sungchun12
Copy link
Contributor

@dbeatty10 Happy to review a PR if you revamp the README for conciseness and cleaner formatting!

@dbeatty10
Copy link
Collaborator Author

@dbeatty10 Happy to review a PR if you revamp the README for conciseness and cleaner formatting!

Thank you @sungchun12 -- created an issue here, and I'll request you as a reviewer if/when I take a swing at a revamp 👍

@dataders
Copy link
Contributor

dataders commented Oct 5, 2022

can you link me to the PR for the change in core?

Here is the PR that added this to Core 1.3.

ok got it! I was hoping this would show up in the docs, but perhaps addressing dbt-labs/docs.getdbt.com#2024 is part of the official 1.3 release milestone?

Copy link
Contributor

@dataders dataders left a comment

Choose a reason for hiding this comment

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

:shipit:

@dataders dataders merged commit de117fb into duckdb Oct 5, 2022
@dataders dataders deleted the dbeatty/docker-instructions branch October 5, 2022 16:46
@dbeatty10
Copy link
Collaborator Author

perhaps addressing dbt-labs/docs.getdbt.com#2024 is part of the official 1.3 release milestone?

🎯

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.

profiles.yml not found with non-default folder name
3 participants