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

[CT-1443] Remove 'root_path' from most node definitions #6171

Closed
gshank opened this issue Oct 28, 2022 · 0 comments · Fixed by #6172
Closed

[CT-1443] Remove 'root_path' from most node definitions #6171

gshank opened this issue Oct 28, 2022 · 0 comments · Fixed by #6172
Assignees
@gshank
Labels
tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality

Comments

@gshank
Copy link
Contributor

gshank commented Oct 28, 2022

We are moving towards enabling manifests to be portable. Currently we include the 'root_path' of the project in every node, which is duplicative and non-portable. In addition, this field is hardly used.

Remove root_path from the node class definitions.

Note: because we don't load seed csv file contents, we do use the root_path to load the seed file when the seeds command is run. For now, we will include the root_path in seed nodes, but longer term we need to come up with a solution for this, whether making it possible to get the root_path for every project in the 'load_agate_table' context method or some other solution.

This is done as part of the structured logging effort, as cleanup of the node attributes that need to be included in the contracts for node information.
@github-actions github-actions bot changed the title Remove 'root_path' from most node definitions [CT-1443] Remove 'root_path' from most node definitions Oct 28, 2022
@gshank gshank added the tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality label Oct 28, 2022
@gshank gshank self-assigned this Oct 28, 2022
@gshank gshank mentioned this issue Oct 28, 2022
Merged
6 tasks
@pratik60 pratik60 mentioned this issue Jan 10, 2023
Merged
@benpankow benpankow mentioned this issue May 16, 2024
Merged
benpankow added a commit to dagster-io/dagster that referenced this issue May 20, 2024
@benpankow
[dagster-dbt] add translator option to attach model files as code ref…
91538f2 
…erence metadata (#21888)

## Summary

Adds the option to a custom `DagsterDbtTranslator` to attach code
reference metadata pointing to the dbt model files for each asset.

Unfortunately, this requires that we also get the `project_dir ` from
the user, since that is only available at run time as part of the
resource. Maybe there's a good way around this? The manifest used to
have a `root_path` field, but this [was recently
removed](dbt-labs/dbt-core#6171).
 
```python
@dbt_assets(
    manifest=...,
    dagster_dbt_translator=DagsterDbtTranslator(
        settings=DagsterDbtTranslatorSettings(attach_sql_model_code_reference=True)
    ),
    project=DbtProject(...)
)
def my_dbt_assets():
  ...
```

## Test Plan

Unit tests.
nikomancy pushed a commit to dagster-io/dagster that referenced this issue May 22, 2024
@benpankow @nikomancy
[dagster-dbt] add translator option to attach model files as code ref…
2aa7847 
…erence metadata (#21888)

## Summary

Adds the option to a custom `DagsterDbtTranslator` to attach code
reference metadata pointing to the dbt model files for each asset.

Unfortunately, this requires that we also get the `project_dir ` from
the user, since that is only available at run time as part of the
resource. Maybe there's a good way around this? The manifest used to
have a `root_path` field, but this [was recently
removed](dbt-labs/dbt-core#6171).
 
```python
@dbt_assets(
    manifest=...,
    dagster_dbt_translator=DagsterDbtTranslator(
        settings=DagsterDbtTranslatorSettings(attach_sql_model_code_reference=True)
    ),
    project=DbtProject(...)
)
def my_dbt_assets():
  ...
```

## Test Plan

Unit tests.
danielgafni pushed a commit to danielgafni/dagster that referenced this issue Jun 18, 2024
@benpankow @danielgafni
[dagster-dbt] add translator option to attach model files as code ref…
fd401bd 
…erence metadata (dagster-io#21888)

## Summary

Adds the option to a custom `DagsterDbtTranslator` to attach code
reference metadata pointing to the dbt model files for each asset.

Unfortunately, this requires that we also get the `project_dir ` from
the user, since that is only available at run time as part of the
resource. Maybe there's a good way around this? The manifest used to
have a `root_path` field, but this [was recently
removed](dbt-labs/dbt-core#6171).
 
```python
@dbt_assets(
    manifest=...,
    dagster_dbt_translator=DagsterDbtTranslator(
        settings=DagsterDbtTranslatorSettings(attach_sql_model_code_reference=True)
    ),
    project=DbtProject(...)
)
def my_dbt_assets():
  ...
```

## Test Plan

Unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gshank gshank

Labels
tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

CT 1443 remove root path dbt-labs/dbt-core
2 participants
@gshank @jtcohen6