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.

Sign up for GitHub

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

Jump to bottom

[Bugfix] Dbt docs operator should not look for graph.gpickle file when --no-write-json is passed. #883

Merged
merged 8 commits into from
Mar 21, 2024
Merged

[Bugfix] Dbt docs operator should not look for graph.gpickle file when --no-write-json is passed. #883

merged 8 commits into from
Mar 21, 2024

Conversation

dwreeves
Copy link
Collaborator

@dwreeves dwreeves commented Mar 9, 2024

Fixes #882

@dwreeves dwreeves requested a review from a team as a code owner March 9, 2024 00:05
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Mar 9, 2024
@dosubot dosubot bot added area:execution Related to the execution environment/mode, like Docker, Kubernetes, Local, VirtualEnv, etc dbt:docs Primarily related to dbt docs command or functionality execution:local Related to Local execution environment labels Mar 9, 2024
@netlify Netlify
Copy link

netlify bot commented Mar 9, 2024 •
edited

✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name Link
🔨 Latest commit f292734
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/65fbf9d0e1bd0f00085e1ced

tatiana
tatiana reviewed Mar 12, 2024
View reviewed changes
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 pointing out and fixing the issue, @dwreeves ! A minor question inline

cosmos/operators/local.py Show resolved Hide resolved
@codecov Codecov
Copy link

codecov bot commented Mar 12, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.40%. Comparing base (ddb25cb) to head (f292734).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #883   +/-   ##
=======================================
  Coverage   95.39%   95.40%           
=======================================
  Files          57       57           
  Lines        2760     2762    +2     
=======================================
+ Hits         2633     2635    +2     
  Misses        127      127

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

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Mar 16, 2024
@dwreeves
Copy link
Collaborator Author

dwreeves commented Mar 16, 2024 •
edited

@tatiana I removed graph.gpickle entirely.

Two tangential notes:

  • I opened Establish general pattern for uploading artifacts to storage #894 to discuss more generally how artifact uploads should be handled.
  • I noticed in the tests that test_operator_execute_deps_parameters does not assert one of its conditions. However when I add the assert, it fails 🤔 This is a separate issue entirely from what I'm trying to do, but I found it curious and felt like I should note it.
image

@dwreeves dwreeves requested a review from tatiana March 16, 2024 15:56
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Mar 18, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 21, 2024
@tatiana tatiana added this to the 1.4.0 milestone Mar 21, 2024
@tatiana tatiana merged commit 4e3a3c8 into astronomer:main Mar 21, 2024
60 checks passed
@tatiana tatiana mentioned this pull request May 2, 2024
Merged
tatiana added a commit that referenced this pull request May 13, 2024
@tatiana
Release 1.4.0 (#934)
7794494 
Features

* Add dbt docs natively in Airflow via plugin by @dwreeves in #737
* Add support for ``InvocationMode.DBT_RUNNER`` for local execution mode
by @jbandoro in #850
* Support partial parsing to render DAGs faster when using
``ExecutionMode.LOCAL``, ``ExecutionMode.VIRTUALENV`` and
``LoadMode.DBT_LS`` by @dwreeves in #800
* Improve performance by 22-35% or more by caching partial parse
artefact by @tatiana in #904
* Add Azure Container Instance as Execution Mode by @danielvdende in
#771
* Add dbt build operators by @dylanharper-qz in #795
* Add dbt profile config variables to mapped profile by @ykuc in #794
* Add more template fields to ``DbtBaseOperator`` by @dwreeves in #786
* Add ``pip_install_options`` argument to operators by @octiva in #808

Bug fixes

* Make ``PostgresUserPasswordProfileMapping`` schema argument optional
by @FouziaTariq in #683
* Fix ``folder_dir`` not showing on logs for ``DbtDocsS3LocalOperator``
by @PrimOox in #856
* Improve ``dbt ls`` parsing resilience to missing tags/config by
@tatiana in #859
* Fix ``operator_args`` modified in place in Airflow converter by
@jbandoro in #835
* Fix Docker and Kubernetes operators execute method resolution by
@jbandoro in #849
* Fix ``TrinoBaseProfileMapping`` required parameter for non method
authentication by @AlexandrKhabarov in #921
* Fix global flags for lists by @ms32035 in #863
* Fix ``GoogleCloudServiceAccountDictProfileMapping`` when getting
values from the Airflow connection ``extra__`` keys by @glebkrapivin in
#923
* Fix using the dag as a keyword argument as ``specific_args_keys`` in
DbtTaskGroup by @tboutaour in #916
* Fix ACI integration (``DbtAzureContainerInstanceBaseOperator``) by
@danielvdende in #872
* Fix setting dbt project dir to the tmp dir by @dwreeves in #873
* Fix dbt docs operator to not use ``graph.gpickle`` file when
``--no-write-json`` is passed by @dwreeves in #883
* Make Pydantic a required dependency by @pankajkoti in #939
* Gracefully error if users try to ``emit_datasets`` with ``Airflow
2.9.0`` or ``2.9.1`` by @tatiana in #948
* Fix parsing tests that have no parents in #933 by @jlaneve
* Correct ``root_path`` in partial parse cache by @pankajkoti in #950

Docs

* Fix docs homepage link by @jlaneve in #860
* Fix docs ``ExecutionConfig.dbt_project_path`` by @jbandoro in #847
* Fix typo in MWAA getting started guide by @jlaneve in #846
* Fix typo related to exporting docs to GCS by @tboutaour in #922
* Improve partial parsing docs by @tatiana in #898
* Improve docs for datasets for airflow >= 2.4 by @SiddiqueAhmad in #879
* Improve test behaviour docs to highlight ``warning`` feature in the
``virtualenv`` mode by @mc51 in #910
* Fix docs typo by @SiddiqueAhmad in #917
* Improve Astro docs by @RNHTTR in #951

Others

* Add performance integration tests by @jlaneve in #827
* Enable ``append_env`` in ``operator_args`` by default by @tatiana in
#899
* Change default ``append_env`` behaviour depending on Cosmos
``ExecutionMode`` by @pankajkoti and @pankajastro in #954
* Expose the ``dbt`` graph in the ``DbtToAirflowConverter`` class by
@tommyjxl in #886
* Improve dbt docs plugin rendering padding by @dwreeves in #876
* Add ``connect_retries`` to databricks profile to fix expensive
integration failures by @jbandoro in #826
* Add import sorting (isort) to Cosmos by @jbandoro in #866
* Add Python 3.11 to CI/tests by @tatiana and @jbandoro in #821, #824
and #825
* Fix failing ``test_created_pod`` for
``apache-airflow-providers-cncf-kubernetes`` after v8.0.0 update by
@jbandoro in #854
* Extend ``DatabricksTokenProfileMapping`` test to include session
properties by @tatiana in #858
* Fix broken integration test uncovered from Pytest 8.0 update by
@jbandoro in #845
* Add Apache Airflow 2.9 to the test matrix by @tatiana in #940
* Replace deprecated ``DummyOperator`` by ``EmptyOperator`` if Airflow
>=2.4.0 by @tatiana in #900
* Improve logs to troubleshoot issue in 1.4.0a2 with astro-cli by
@tatiana in #947
* Fix issue when publishing a new release to PyPI by @tatiana in #946
* Pre-commit hook updates in #820, #834, #843 and #852, #890, #896,
#901, #905, #908, #919, #931, #941
@tatiana tatiana mentioned this pull request May 14, 2024
Merged
tatiana added a commit that referenced this pull request May 15, 2024
@tatiana
Promote @dwreeves to committer (#960)
d791d20 
[Daniel Reeves](https://www.linkedin.com/in/daniel-reeves-27700545/)
(@dwreeves ) is an experienced Open-Source Developer currently working
as a Data Architect at Battery Ventures. He has significant experience
with Apache Airflow, SQL, and Python and has contributed to many [OSS
projects](https://github.com/dwreeve).

Not only has he been using Cosmos since its early stages, but since
January 2023, he has actively contributed to the project:
![Screenshot 2024-05-14 at 10 47
30](https://github.com/astronomer/astronomer-cosmos/assets/272048/57829cb6-7eee-4b02-998b-46cc7746f15a)

He has been a critical driver for the Cosmos 1.4 release, and some of
his contributions include new features, bug fixes, and documentation
improvements, including:
* Creation of an Airflow plugin to render dbt docs:
#737
* Support using dbt partial parsing file:
#800
* Add more template fields to `DbtBaseOperator`:
#786
* Add cancel on kill functionality:
#101
* Make region optional in Snowflake profile mapping:
#100
* Fix the dbt docs operator to not look for `graph.pickle`:
#883

He thinks about the project long-term and proposes thorough solutions to
problems faced by the community, as can be seen in Github tickets:
* Introducing composability in the middle layer of Cosmos's API:
#895
* Establish a general pattern for uploading artifacts to storage:
#894
* Support `operator_arguments` injection at a node level:
#881

One of Daniel's notable traits is his collaborative and supportive
approach. He has actively engaged with users in the #airflow-dbt Slack
channel, demonstrating his commitment to fostering a supportive
community.

We want to promote him as a Cosmos committer and maintainer for all
these, recognising his constant efforts and achievements towards our
community. Thank you very much, @dwreeves !
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tatiana tatiana tatiana approved these changes

Assignees
No one assigned
Labels
area:execution Related to the execution environment/mode, like Docker, Kubernetes, Local, VirtualEnv, etc dbt:docs Primarily related to dbt docs command or functionality execution:local Related to Local execution environment lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.

[Bug] Dbt docs operator should not look for graph.gpickle file when --no-write-json is passed.
2 participants
@dwreeves @tatiana