Skip to content

Commit

Permalink
[components] Remove Definitions.merge_internal. Use suppress_dagster_…
Browse files Browse the repository at this point in the history
…warnings (#26218)

## Summary & Motivation

Addressing feedback in #26201

* Use `suppress_dagster_warnings` instead of `warnings`
* Do not have `Definitions.merge_internal`.

## How I Tested These Changes

BK
  • Loading branch information
schrockn authored Dec 2, 2024
1 parent 0bbf1d0 commit f064c92
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
find_component_decl,
)
from dagster._components.core.deployment import CodeLocationProjectContext
from dagster._utils.warnings import suppress_dagster_warnings

if TYPE_CHECKING:
from dagster._core.definitions.definitions_class import Definitions
Expand Down Expand Up @@ -51,6 +52,7 @@ def build_defs_from_component_folder(
return defs_from_components(resources=resources, context=context, components=components)


@suppress_dagster_warnings
def defs_from_components(
*,
context: ComponentLoadContext,
Expand All @@ -59,12 +61,13 @@ def defs_from_components(
) -> "Definitions":
from dagster._core.definitions.definitions_class import Definitions

return Definitions.merge_internal(
[*[c.build_defs(context) for c in components], Definitions(resources=resources)]
return Definitions.merge(
*[*[c.build_defs(context) for c in components], Definitions(resources=resources)]
)


# Public method so optional Nones are fine
@suppress_dagster_warnings
def build_defs_from_toplevel_components_folder(
path: Path,
resources: Optional[Mapping[str, object]] = None,
Expand All @@ -84,4 +87,4 @@ def build_defs_from_toplevel_components_folder(
resources=resources or {},
)
all_defs.append(defs)
return Definitions.merge_internal(all_defs)
return Definitions.merge(*all_defs)
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import shutil
import warnings
from pathlib import Path
from typing import TYPE_CHECKING, Any, Mapping, Optional, Sequence

Expand All @@ -13,7 +12,7 @@
from dagster._core.definitions.decorators.asset_decorator import multi_asset
from dagster._core.execution.context.asset_execution_context import AssetExecutionContext
from dagster._core.pipes.subprocess import PipesSubprocessClient
from dagster._utils.warnings import ExperimentalWarning
from dagster._utils.warnings import suppress_dagster_warnings

if TYPE_CHECKING:
from dagster._core.definitions.definitions_class import Definitions
Expand All @@ -30,15 +29,14 @@ class AssetSpecModel(BaseModel):
owners: Sequence[str] = []
tags: Mapping[str, str] = {}

@suppress_dagster_warnings
def to_asset_spec(self) -> AssetSpec:
with warnings.catch_warnings():
warnings.simplefilter("ignore", category=ExperimentalWarning)
return AssetSpec(
**{
**self.__dict__,
"key": AssetKey.from_user_string(self.key),
},
)
return AssetSpec(
**{
**self.__dict__,
"key": AssetKey.from_user_string(self.key),
},
)


class PipesSubprocessScriptParams(BaseModel):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,11 +635,6 @@ def merge(*def_sets: "Definitions") -> "Definitions":
Returns:
Definitions: The merged definitions.
"""
check.sequence_param(def_sets, "def_sets", of_type=Definitions)
return Definitions.merge_internal(def_sets)

@staticmethod
def merge_internal(def_sets: Sequence["Definitions"]) -> "Definitions":
check.sequence_param(def_sets, "def_sets", of_type=Definitions)
assets = []
schedules = []
Expand Down

0 comments on commit f064c92

Please sign in to comment.