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

[dbt] Add DbtManifestPreparer and DagsterDbtManifestPreparer apidoc #22546

Merged
merged 6 commits into from
Jun 27, 2024
Merged

[dbt] Add DbtManifestPreparer and DagsterDbtManifestPreparer apidoc #22546

merged 6 commits into from
Jun 27, 2024

Conversation

maximearmstrong
Copy link
Contributor

@maximearmstrong maximearmstrong commented Jun 13, 2024 •
edited
Loading

Summary & Motivation

This PR updates the docstring for DbtManifestPreparer and DagsterDbtManifestPreparer.

How I Tested These Changes

make apidoc-build

This was referenced Jun 13, 2024
Merged
Merged
@maximearmstrong Graphite App
Copy link
Contributor Author

maximearmstrong commented Jun 13, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @maximearmstrong and the rest of your teammates on Graphite Graphite

@maximearmstrong maximearmstrong mentioned this pull request Jun 13, 2024
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 13, 2024 •
edited
Loading

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-esccjz654-elementl.vercel.app
https://maxime-ds-314-add-docstring-for-dbtmanifestpreparer.dagster.dagster-docs.io

Direct link to changed pages:

@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from c83e06a to 9429439 Compare June 14, 2024 19:52
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from a09221a to 86b50b6 Compare June 14, 2024 19:52
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 9429439 to 177ecf1 Compare June 14, 2024 20:00
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 86b50b6 to 9a47079 Compare June 14, 2024 20:01
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 177ecf1 to 5a38e7f Compare June 14, 2024 20:24
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 9a47079 to a76d425 Compare June 14, 2024 20:24
@maximearmstrong maximearmstrong self-assigned this Jun 14, 2024
@maximearmstrong maximearmstrong marked this pull request as ready for review June 14, 2024 20:47
@graphite-app graphite-app bot added the area: docs Related to documentation in general label Jun 14, 2024
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 5a38e7f to 6391377 Compare June 17, 2024 19:50
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from cf5d5e9 to 38026a4 Compare June 17, 2024 19:50
@maximearmstrong maximearmstrong mentioned this pull request Jun 17, 2024
Merged
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 6391377 to 68055dc Compare June 17, 2024 20:38
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 38026a4 to f64c377 Compare June 17, 2024 20:38
@maximearmstrong maximearmstrong mentioned this pull request Jun 17, 2024
Merged
3 tasks
sryza
sryza suggested changes Jun 17, 2024
View reviewed changes
python_modules/libraries/dagster-dbt/dagster_dbt/dbt_project.py
@@ -55,7 +66,19 @@ def __init__(
"""
self._generate_cli_args = generate_cli_args or ["parse", "--quiet"]

@public
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the other names used here, should this be named something like prepare_if_local? Or ensure_prepared?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensure_prepared would make sense. I think making the change in the PR updating the API would make most sense.

@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 68055dc to 339f8fb Compare June 18, 2024 18:21
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from f64c377 to 018aeac Compare June 18, 2024 18:21
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 339f8fb to 4ed43a9 Compare June 18, 2024 20:51
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 018aeac to 3a9ab61 Compare June 18, 2024 20:51
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 4ed43a9 to c3e9555 Compare June 18, 2024 21:40
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 3a9ab61 to c6732c3 Compare June 18, 2024 21:40
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 12ca547 to e8575c4 Compare June 21, 2024 17:17
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from f27b600 to b8f8d86 Compare June 21, 2024 19:00
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from e8575c4 to 69ecf3a Compare June 21, 2024 19:00
@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from b8f8d86 to 058e02d Compare June 21, 2024 19:52
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 69ecf3a to 78a86f0 Compare June 21, 2024 19:52
@maximearmstrong maximearmstrong mentioned this pull request Jun 21, 2024
Merged
sryza
sryza previously requested changes Jun 24, 2024
View reviewed changes
Copy link
Contributor

@sryza sryza left a comment

Choose a reason for hiding this comment

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

Do we still want to expose these, given our latest conversation?

@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from 058e02d to e4d7625 Compare June 24, 2024 19:09
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 78a86f0 to 200c8db Compare June 24, 2024 19:09
@maximearmstrong
Copy link
Contributor Author

Do we still want to expose these, given our latest conversation?

@sryza That's a good point. We should not expose these.

Though, I'm in favor of keeping the new docstring as a reference in the code.

@sryza
Copy link
Contributor

sryza commented Jun 24, 2024

Though, I'm in favor of keeping the new docstring as a reference in the code.

I'm also very in favor of that.

@maximearmstrong maximearmstrong force-pushed the maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental branch from e4d7625 to 89b5c7b Compare June 26, 2024 17:16
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch 2 times, most recently from 71c3f22 to b26e050 Compare June 26, 2024 17:39
@maximearmstrong maximearmstrong changed the base branch from maxime/ds-315/make-attribute-for-dbtmanifestpreparer-experimental to maxime/rename-cli-endpoint-for-dbt-project June 26, 2024 17:39
sryza
sryza approved these changes Jun 26, 2024
View reviewed changes
Copy link
Contributor

@sryza sryza left a comment

Choose a reason for hiding this comment

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

Nice

@maximearmstrong maximearmstrong force-pushed the maxime/rename-cli-endpoint-for-dbt-project branch from b7ccaee to e574801 Compare June 27, 2024 13:43
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from b26e050 to 5f31de0 Compare June 27, 2024 13:43
Base automatically changed from maxime/rename-cli-endpoint-for-dbt-project to master June 27, 2024 14:22
@maximearmstrong maximearmstrong force-pushed the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch from 5f31de0 to 0ea9a4f Compare June 27, 2024 14:56
@maximearmstrong maximearmstrong merged commit 684409b into master Jun 27, 2024
2 checks passed
@maximearmstrong maximearmstrong deleted the maxime/ds-314/add-docstring-for-dbtmanifestpreparer branch June 27, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sryza sryza sryza approved these changes

@rexledesma rexledesma Awaiting requested review from rexledesma

Assignees

@maximearmstrong maximearmstrong

Labels
area: docs Related to documentation in general
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maximearmstrong @sryza