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 dlt asset metadata via translator #22047

Merged
merged 3 commits into from
May 23, 2024

Conversation

edsoncezar16
Copy link
Contributor

Summary & Motivation

Asset metadata is an essential component of assets definition in real projects. This PR adds a parameter in the DagsterDltTranslator to define resource specific metadata.

This is an idea to circumvent the fact that dlt does nor uses a configuration file such as Sling's replication.yaml, where would be natural to define metadata to be read by the translator.

How I Tested These Changes

make pyright
make ruff
python -m pytest python_modules/libraries/dagster-embedded-elt/dagster_embedded_elt_tests/dlt_tests

@cmpadden cmpadden self-requested a review May 23, 2024 14:10
@@ -11,6 +11,8 @@

@dataclass
class DagsterDltTranslator:
metadata_by_resource_name: Optional[Mapping[str, Any]] = None
Copy link
Member

Choose a reason for hiding this comment

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

Feels weird to have this in the base DagsterDltTranslator class. If a user wants to have this, you can subclass DagsterDltTranslator and add it yourself as a property. [1]

Copy link
Member

@rexledesma rexledesma left a comment

Choose a reason for hiding this comment

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

Looks good! Going to run tests before merging.

@cmpadden cmpadden merged commit f7e13af into dagster-io:master May 23, 2024
1 check passed
@edsoncezar16 edsoncezar16 deleted the feature/dlt-assets-metadata branch May 30, 2024 22:26
danielgafni pushed a commit to danielgafni/dagster that referenced this pull request Jun 18, 2024
## Summary & Motivation

Asset metadata is an essential component of assets definition in real
projects. This PR adds a parameter in the DagsterDltTranslator to define
resource specific metadata.

This is an idea to circumvent the fact that dlt does nor uses a
configuration file such as Sling's replication.yaml, where would be
natural to define metadata to be read by the translator.

## How I Tested These Changes

make pyright
make ruff
python -m pytest
python_modules/libraries/dagster-embedded-elt/dagster_embedded_elt_tests/dlt_tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants