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

Enable Docs DAG in CI leveraging existing CI connections #1428

Merged
merged 7 commits into from
Dec 27, 2024
Merged

Conversation

pankajkoti
Copy link
Contributor

@pankajkoti pankajkoti commented Dec 26, 2024

It seems that the dbt docs DAG in our example DAGs suite was silently failing to run because the specified connections in the DAG were not present in our CI environment. I am removing the connection checks, allowing the previously skipped tasks to execute even when the remote Airflow connections are unavailable. Additionally, I’ve updated the DAG to use our existing CI credentials and Airflow connections, as well as updated the bucket name to the ones I created in our object stores. This change will help catch issues like #1420 earlier. While fix #1422 has already addressed #1420, this PR will now validate that the fix is functioning as expected.

closes: #1420

Copy link

netlify bot commented Dec 26, 2024

Deploy Preview for sunny-pastelito-5ecb04 ready!

Name Link
🔨 Latest commit bdde29f
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/676eb522415fe600087d6dda
😎 Deploy Preview https://deploy-preview-1428--sunny-pastelito-5ecb04.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

cloudflare-workers-and-pages bot commented Dec 26, 2024

Deploying astronomer-cosmos with  Cloudflare Pages  Cloudflare Pages

Latest commit: bdde29f
Status: ✅  Deploy successful!
Preview URL: https://d780da1a.astronomer-cosmos.pages.dev
Branch Preview URL: https://enable-docs-dag.astronomer-cosmos.pages.dev

View logs

@pankajkoti pankajkoti marked this pull request as ready for review December 26, 2024 07:00
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 26, 2024
@dosubot dosubot bot added the area:ci Related to CI, Github Actions, or other continuous integration tools label Dec 26, 2024
@pankajkoti pankajkoti requested a review from tatiana December 26, 2024 07:00
@pankajkoti
Copy link
Contributor Author

Action run https://github.com/astronomer/astronomer-cosmos/actions/runs/12500718837/job/34877247783?pr=1428 before merging and rebasing PR #1422 here shows that we now are running the example DAG for docs upload test in our CI with this PR.

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.

Thanks for addressing this, @pankajkoti ! It seems the CI is failing for other reasons. Happy for this to be merged once we address the other issues in a separate PR - so the CI is green.

dev/dags/dbt_docs.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.92%. Comparing base (0f89e01) to head (bdde29f).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1428      +/-   ##
==========================================
+ Coverage   96.52%   96.92%   +0.39%     
==========================================
  Files          73       73              
  Lines        4320     4320              
==========================================
+ Hits         4170     4187      +17     
+ Misses        150      133      -17     

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

@pankajkoti
Copy link
Contributor Author

Thanks for addressing this, @pankajkoti ! It seems the CI is failing for other reasons. Happy for this to be merged once we address the other issues in a separate PR - so the CI is green.

thanks @tatiana . There were a couple of issues. One of them was resolved in PR #1432 . The other being that the DAG operators were leading to compilation errors as dbt deps were not installed, which led to failing command execution and hence no target directory was created from where we were trying to upload the files. We now are installing dbt deps in the DAG operators.

@pankajkoti pankajkoti requested a review from tatiana December 27, 2024 14:38
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.

Thanks for fixing all the issues, @pankajkoti !

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 27, 2024
@tatiana tatiana merged commit 050ecd4 into main Dec 27, 2024
69 checks passed
@tatiana tatiana deleted the enable-docs-dag branch December 27, 2024 16:44
@pankajastro pankajastro mentioned this pull request Dec 30, 2024
pankajastro added a commit that referenced this pull request Dec 30, 2024
1.8.1 (2024-12-30)
--------------------

Bug Fixes

* Fix rendering dbt tests with multiple parents by @tatiana in #1433
* Add ``kwargs`` param in DocsOperator method
``upload_to_cloud_storage`` by @pankajastro in #1422

Docs

* Improve OpenLineage documentation by @tatiana in #1431

Others

* Enable Docs DAG in CI leveraging existing CI connections by
@pankajkoti in #1428
* Install providers with airflow by @pankajkoti in #1432
* Remove unused docs dependency by @pankajastro in #1414
* Pre-commit hook updates in #1424 

---------

Co-authored-by: Tatiana Al-Chueyr <[email protected]>
Co-authored-by: Pankaj Koti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ci Related to CI, Github Actions, or other continuous integration tools lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] DbtDocsGCSLocalOperator.upload_to_cloud_storage() got an unexpected keyword argument 'context'
2 participants