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

Update getting started guide with an example dbt project #778

Closed
wants to merge 3 commits into from

Conversation

RNHTTR
Copy link
Contributor

@RNHTTR RNHTTR commented Jan 2, 2024

Description

I was going through the getting started guide, and it took me a little while to find a simple getting started project that I could use to demo Cosmos.

Related Issue(s)

Breaking Change?

Checklist

  • I have made corresponding changes to the documentation (if required)
  • I have added tests that prove my fix is effective or that my feature works

@RNHTTR RNHTTR requested a review from a team as a code owner January 2, 2024 21:50
@RNHTTR RNHTTR requested a review from a team January 2, 2024 21:50
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jan 2, 2024
Copy link

netlify bot commented Jan 2, 2024

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit e7609a5
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/659560be906ac60008fc1359

@dosubot dosubot bot added area:docs Relating to documentation, changes, fixes, improvement dbt:docs Primarily related to dbt docs command or functionality labels Jan 2, 2024
Copy link

codecov bot commented Jan 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fd729ff) 93.28% compared to head (e7609a5) 93.28%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #778   +/-   ##
=======================================
  Coverage   93.28%   93.28%           
=======================================
  Files          55       55           
  Lines        2502     2502           
=======================================
  Hits         2334     2334           
  Misses        168      168           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

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

Hi @RNHTTR, thanks for pointing this out! There is room for us to improve the onboarding of Cosmos in Astro.

Some broader questions before we merge this PR (please, @jlaneve & @harels, share your thoughts!)

  1. The last versions of dbt and Airflow don't conflict (https://astronomer.github.io/astronomer-cosmos/getting_started/execution-modes-local-conflicts.html#execution-modes-local-conflicts). Do we still want to encourage Astro users to create an independent virtualenv for dbt when using Cosmos? Providers/adapters may conflict, so this may be the safest option. Please let me know what your thoughts are. Unfortunately since the recent Airflow 2.8 release, we're back to having several conflicts between Airflow (2.8) & dbt (1.0-1.4, 1.6, 1.7): https://github.com/astronomer/astronomer-cosmos/pull/779/files So the answer to this question is yes, we should stick to the separate virtualenv for dbt for now.

  2. What is the goal of this getting started? Is it a step-by-step for someone trying out Cosmos and Astro for the first time? This Astro+Cosmos demo by @jlaneve is great:
    https://github.com/astronomer/cosmos-demo
    It is straightforward, self-contained, and uses Jaffle Shop. Would it make sense to use it / reference it?

- A dbt project. The `jaffle shop example <https://github.com/dbt-labs/jaffle_shop>`_ is a great first step. To use the jaffle shop example to demo Cosmos...

- Clone the jaffle shop project using ``git clone`` into the ``/dags`` directory created by ``astro dev init``
- Add ``dbt-postgres`` to the requirements.txt file created by ``astro dev init``
Copy link
Collaborator

Choose a reason for hiding this comment

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

@RNHTTR in this example, dbt is being installed in an independent Python virtual env, different from Airflow's Python deps.

Therefore, this dependency should be in the Create a virtualenv environment section, in the Dockerfile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the goal of this getting started?

I didn't create it! I just found it. I'm likely in the minority for cosmos users, but I don't have dbt experience and I don't have a dbt project laying around, so the motivation for this change was to include a sample dbt project. @jlaneve 's cosmos-demo works nicely -- I hadn't seen that before. I think the docs should link to that though maybe?

The current "quickstart" link just goes to the cosmos docs

tatiana added a commit that referenced this pull request May 10, 2024
Based on:
#778

Co-authored-by: Ryan Hatter <25823361+RNHTTR@users.noreply.github.com>
tatiana added a commit that referenced this pull request May 10, 2024
Based on: #778

Co-authored-by: Ryan Hatter <25823361+RNHTTR@users.noreply.github.com>
Co-authored-by: Pankaj Koti <pankaj.koti@astronomer.io>
@tatiana
Copy link
Collaborator

tatiana commented May 10, 2024

Closed after minor docs changes: #951

@tatiana tatiana closed this May 10, 2024
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
Based on: astronomer#778

Co-authored-by: Ryan Hatter <25823361+RNHTTR@users.noreply.github.com>
Co-authored-by: Pankaj Koti <pankaj.koti@astronomer.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:docs Relating to documentation, changes, fixes, improvement dbt:docs Primarily related to dbt docs command or functionality size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants