Skip to content

Commit

Permalink
Revert "Try to make dataset objects totally unhashable (apache#42054)" (
Browse files Browse the repository at this point in the history
apache#42065)

This reverts commit 1c4a00b.
  • Loading branch information
potiuk committed Sep 6, 2024
1 parent caa94fa commit ce3df19
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 17 deletions.
22 changes: 19 additions & 3 deletions airflow/datasets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,20 @@ def iter_dag_dependencies(self, *, source: str, target: str) -> Iterator[DagDepe
raise NotImplementedError


@attr.define(unsafe_hash=False)
@attr.define()
class DatasetAlias(BaseDataset):
"""A represeation of dataset alias which is used to create dataset during the runtime."""

name: str

def __eq__(self, other: Any) -> bool:
if isinstance(other, DatasetAlias):
return self.name == other.name
return NotImplemented

def __hash__(self) -> int:
return hash(self.name)

def iter_dag_dependencies(self, *, source: str, target: str) -> Iterator[DagDependency]:
"""
Iterate a dataset alias as dag dependency.
Expand All @@ -233,21 +241,29 @@ class DatasetAliasEvent(TypedDict):
dest_dataset_uri: str


@attr.define(unsafe_hash=False)
@attr.define()
class Dataset(os.PathLike, BaseDataset):
"""A representation of data dependencies between workflows."""

uri: str = attr.field(
converter=_sanitize_uri,
validator=[attr.validators.min_len(1), attr.validators.max_len(3000)],
)
extra: dict[str, Any] = attr.field(factory=dict)
extra: dict[str, Any] | None = None

__version__: ClassVar[int] = 1

def __fspath__(self) -> str:
return self.uri

def __eq__(self, other: Any) -> bool:
if isinstance(other, self.__class__):
return self.uri == other.uri
return NotImplemented

def __hash__(self) -> int:
return hash(self.uri)

@property
def normalized_uri(self) -> str | None:
"""
Expand Down
2 changes: 1 addition & 1 deletion airflow/lineage/hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def create_dataset(
"""
if uri:
# Fallback to default factory using the provided URI
return Dataset(uri=uri, extra=dataset_extra or {})
return Dataset(uri=uri, extra=dataset_extra)

if not scheme:
self.log.debug(
Expand Down
4 changes: 2 additions & 2 deletions airflow/models/dag.py
Original file line number Diff line number Diff line change
Expand Up @@ -2786,12 +2786,12 @@ def bulk_write_to_db(
curr_outlet_references = curr_orm_dag and curr_orm_dag.task_outlet_dataset_references
for task in dag.tasks:
dataset_outlets: list[Dataset] = []
dataset_alias_outlets: list[DatasetAlias] = []
dataset_alias_outlets: set[DatasetAlias] = set()
for outlet in task.outlets:
if isinstance(outlet, Dataset):
dataset_outlets.append(outlet)
elif isinstance(outlet, DatasetAlias):
dataset_alias_outlets.append(outlet)
dataset_alias_outlets.add(outlet)

if not dataset_outlets:
if curr_outlet_references:
Expand Down
4 changes: 0 additions & 4 deletions newsfragments/42054.significant.rst

This file was deleted.

14 changes: 10 additions & 4 deletions tests/datasets/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ def test_not_equal_when_different_uri():
assert dataset1 != dataset2


def test_hash():
uri = "s3://example/dataset"
dataset = Dataset(uri=uri)
hash(dataset)


def test_dataset_logic_operations():
result_or = dataset1 | dataset2
assert isinstance(result_or, DatasetAny)
Expand Down Expand Up @@ -181,10 +187,10 @@ def test_datasetbooleancondition_evaluate_iter():
assert all_condition.evaluate({"s3://bucket1/data1": True, "s3://bucket2/data2": False}) is False

# Testing iter_datasets indirectly through the subclasses
datasets_any = dict(any_condition.iter_datasets())
datasets_all = dict(all_condition.iter_datasets())
assert datasets_any == {"s3://bucket1/data1": dataset1, "s3://bucket2/data2": dataset2}
assert datasets_all == {"s3://bucket1/data1": dataset1, "s3://bucket2/data2": dataset2}
datasets_any = set(any_condition.iter_datasets())
datasets_all = set(all_condition.iter_datasets())
assert datasets_any == {("s3://bucket1/data1", dataset1), ("s3://bucket2/data2", dataset2)}
assert datasets_all == {("s3://bucket1/data1", dataset1), ("s3://bucket2/data2", dataset2)}


@pytest.mark.parametrize(
Expand Down
4 changes: 2 additions & 2 deletions tests/lineage/test_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_add_input_dataset(self, mock_dataset):
self.collector.add_input_dataset(hook, uri="test_uri")

assert next(iter(self.collector._inputs.values())) == (dataset, hook)
mock_dataset.assert_called_once_with(uri="test_uri", extra={})
mock_dataset.assert_called_once_with(uri="test_uri", extra=None)

def test_grouping_datasets(self):
hook_1 = MagicMock()
Expand All @@ -96,7 +96,7 @@ def test_grouping_datasets(self):
@patch("airflow.lineage.hook.ProvidersManager")
def test_create_dataset(self, mock_providers_manager):
def create_dataset(arg1, arg2="default", extra=None):
return Dataset(uri=f"myscheme://{arg1}/{arg2}", extra=extra or {})
return Dataset(uri=f"myscheme://{arg1}/{arg2}", extra=extra)

mock_providers_manager.return_value.dataset_factories = {"myscheme": create_dataset}
assert self.collector.create_dataset(
Expand Down
2 changes: 1 addition & 1 deletion tests/timetables/test_datasets_timetable.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_serialization(dataset_timetable: DatasetOrTimeSchedule, monkeypatch: An
"timetable": "mock_serialized_timetable",
"dataset_condition": {
"__type": "dataset_all",
"objects": [{"__type": "dataset", "uri": "test_dataset", "extra": {}}],
"objects": [{"__type": "dataset", "uri": "test_dataset", "extra": None}],
},
}

Expand Down

0 comments on commit ce3df19

Please sign in to comment.