Skip to content

Conversation

@lucargir
Copy link
Contributor

@lucargir lucargir commented Nov 6, 2025

This would close #54.

  • Convert SQLMeshDagsterTranslator to extend ConfigurableResource for Dagster resource injection pattern
  • Change IntermediateAssetOut and IntermediateAssetDep from Pydantic BaseModel to dataclass for lighter weight
  • Use AssetKey instead of AssetDep for external dependencies to prevent duplicate asset definitions in containerized code locations
  • Remove translator attribute from SQLMeshResource; use config.get_translator() as single source of truth
  • Simplify docstrings throughout

@lucargir
Copy link
Contributor Author

lucargir commented Nov 6, 2025

@ravenac95 I was also checking the ConvertibleToAssetOut and the IntermediateAssetOut, and I am not sure why are these used. If it's to be able to use SQLMesh' cache, shouldn't we just update the config to include the path to the cache directory and instantiate the context with that custom setting?

From their python API docs, it shows that it's defined as ".cache" by default, so not sure if this custom intermediate asset is needed. Or is this because if the AssetOut gets created during the initialisation of Dagster's code location, it takes a long time?

@ravenac95
Copy link
Member

ravenac95 commented Nov 8, 2025

@ravenac95 I was also checking the ConvertibleToAssetOut and the IntermediateAssetOut, and I am not sure why are these used. If it's to be able to use SQLMesh' cache, shouldn't we just update the config to include the path to the cache directory and instantiate the context with that custom setting?

From their python API docs, it shows that it's defined as ".cache" by default, so not sure if this custom intermediate asset is needed. Or is this because if the AssetOut gets created during the initialisation of Dagster's code location, it takes a long time?

Ah yes this is a good question. We actually had to do this because we do some work with SQLMesh that requires essentially model generation with python models (this can take some significant time). This isn't something inherently handled by SQLMesh so in some weird way this is a pretty specific use case we needed to support for ourselves. At the moment we don't plan on changing any of that, but it's possible we could pull more of this into our own repository as opposed to leaning on the integration. So for now can we update this to keep the IntermediateAssetOut?

@lucargir lucargir force-pushed the CoercibleToAssetDep branch 2 times, most recently from ed11df6 to eeff2b2 Compare January 12, 2026 12:23
@lucargir lucargir changed the title feat: change from ConvertibleToAssetDep to CoercibleToAssetDep feat: change from ConvertibleToAssetDep to ConvertibleToAssetKey Jan 12, 2026
@lucargir lucargir force-pushed the CoercibleToAssetDep branch from eeff2b2 to af31660 Compare January 12, 2026 18:00
@lucargir lucargir closed this Jan 12, 2026
@lucargir lucargir force-pushed the CoercibleToAssetDep branch from af31660 to cd56c58 Compare January 12, 2026 18:00
…nal deps

- Convert SQLMeshDagsterTranslator to extend ConfigurableResource for
  Dagster resource injection pattern
- Change IntermediateAssetOut and IntermediateAssetDep from Pydantic
  BaseModel to dataclass for lighter weight
- Use AssetKey instead of AssetDep for external dependencies to prevent
  duplicate asset definitions in containerized code locations
- Remove translator attribute from SQLMeshResource; use config.get_translator()
  as single source of truth
- Simplify docstrings throughout
@lucargir lucargir reopened this Jan 13, 2026
@lucargir
Copy link
Contributor Author

@ravenac95 just made an updated PR for this, I think this should work with the requirements that you have, but still be flexible enough to fix the issue with external deps.

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.

Change from ConvertibleToAssetDep to CoercibleToAssetDep

2 participants