From 07d8b5c17a7b5dfb7f8eb68d2aa1d6fc492f5cd9 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Wed, 3 Sep 2025 09:59:45 +0530 Subject: [PATCH 01/10] Replicated the issue with tests --- tests/unit/test_installation.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/test_installation.py b/tests/unit/test_installation.py index 2eb6811..bf50b32 100644 --- a/tests/unit/test_installation.py +++ b/tests/unit/test_installation.py @@ -761,3 +761,19 @@ class ForwardReferencingClass: loaded = installation.load(ForwardReferencingClass, filename="saved.yml") assert instance == loaded + +def test_missing_attribute() -> None: + + @dataclass + class MissingAttributeClass: + __file__ = "config.yml" + __version__ = 3 + skip_validation: bool = False + sdk_config: JsonValue = None + + instance = MissingAttributeClass(False, {"warehouse_id": "8xc123456"}) + installation = MockInstallation() + installation.save(instance) + + loaded = installation.load(MissingAttributeClass) + assert instance == loaded From 54a6e19c988c80c9f376a2e74d28284aeaf65d10 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Mon, 22 Sep 2025 11:37:25 +0530 Subject: [PATCH 02/10] Add support for correct serialization and deserialization of bool type fields in dataclass objects --- src/databricks/labs/blueprint/installation.py | 3 ++- tests/unit/test_installation.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/databricks/labs/blueprint/installation.py b/src/databricks/labs/blueprint/installation.py index fdb573f..02412b3 100644 --- a/src/databricks/labs/blueprint/installation.py +++ b/src/databricks/labs/blueprint/installation.py @@ -632,7 +632,8 @@ def _marshal_dataclass(self, type_ref: type, path: list[str], inst: Any) -> tupl value, ok = self._marshal(hint, [*path, field], raw) if not ok: raise SerdeError(self._explain_why(hint, [*path, field], raw)) - if not value: + # Earlier for boolean fields when the value was False, we would skip it. Which was incorrect + if value is None: continue as_dict[field] = value return as_dict, True diff --git a/tests/unit/test_installation.py b/tests/unit/test_installation.py index bf50b32..e4e3c13 100644 --- a/tests/unit/test_installation.py +++ b/tests/unit/test_installation.py @@ -762,13 +762,13 @@ class ForwardReferencingClass: loaded = installation.load(ForwardReferencingClass, filename="saved.yml") assert instance == loaded -def test_missing_attribute() -> None: +def test_bool_attribute() -> None: @dataclass class MissingAttributeClass: __file__ = "config.yml" __version__ = 3 - skip_validation: bool = False + skip_validation: bool = True sdk_config: JsonValue = None instance = MissingAttributeClass(False, {"warehouse_id": "8xc123456"}) From a4c6ae7f35943169fdc4659149cca126457bbf99 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Mon, 22 Sep 2025 11:42:05 +0530 Subject: [PATCH 03/10] fmt fixes --- tests/unit/test_installation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_installation.py b/tests/unit/test_installation.py index e4e3c13..f475145 100644 --- a/tests/unit/test_installation.py +++ b/tests/unit/test_installation.py @@ -762,8 +762,8 @@ class ForwardReferencingClass: loaded = installation.load(ForwardReferencingClass, filename="saved.yml") assert instance == loaded -def test_bool_attribute() -> None: +def test_bool_attribute() -> None: @dataclass class MissingAttributeClass: __file__ = "config.yml" From cc204ca42a48f045185fb949a0d38391d86b4323 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Mon, 22 Sep 2025 11:46:45 +0530 Subject: [PATCH 04/10] fix hatch click dependency --- .github/workflows/acceptance.yml | 2 +- .github/workflows/downstreams.yml | 2 +- .github/workflows/nightly.yml | 2 +- .github/workflows/push.yml | 5 ++--- .github/workflows/release.yml | 2 +- 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/acceptance.yml b/.github/workflows/acceptance.yml index 92e118f..9ffafdc 100644 --- a/.github/workflows/acceptance.yml +++ b/.github/workflows/acceptance.yml @@ -29,7 +29,7 @@ jobs: python-version: '3.10' - name: Install Hatch - run: pip install hatch==1.9.4 + run: pip install hatch==1.9.4 'click<8.3.0' # https://github.com/pallets/click/issues/3065 - name: Acceptance uses: databrickslabs/sandbox/acceptance@acceptance/v0.4.2 diff --git a/.github/workflows/downstreams.yml b/.github/workflows/downstreams.yml index ecf93e8..d07d7b0 100644 --- a/.github/workflows/downstreams.yml +++ b/.github/workflows/downstreams.yml @@ -44,7 +44,7 @@ jobs: - name: Install toolchain run: | - pip install hatch==1.9.4 + pip install hatch==1.9.4 'click<8.3.0' # https://github.com/pallets/click/issues/3065 - name: Acceptance uses: databrickslabs/sandbox/downstreams@acceptance/v0.4.2 diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 4a24977..8f2a274 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -32,7 +32,7 @@ jobs: python-version: '3.10' - name: Install hatch - run: pip install hatch==1.9.4 + run: pip install hatch==1.9.4 'click<8.3.0' # https://github.com/pallets/click/issues/3065 - name: Run nightly tests uses: databrickslabs/sandbox/acceptance@acceptance/v0.4.2 diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 5ff14fa..bd18c11 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -36,11 +36,10 @@ jobs: python-version: ${{ matrix.pyVersion }} - name: Install hatch - run: pip install hatch==$HATCH_VERSION + run: pip install hatch==$HATCH_VERSION 'click<8.3.0' # https://github.com/pallets/click/issues/3065 - name: Run unit tests run: | - pip install hatch==1.9.4 make test - name: Publish test coverage @@ -61,7 +60,7 @@ jobs: python-version: '3.10' - name: Install hatch - run: pip install hatch==$HATCH_VERSION + run: pip install hatch==$HATCH_VERSION 'click<8.3.0' # https://github.com/pallets/click/issues/3065 - name: Format all files run: make dev fmt diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 528e025..5d39d4a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -27,7 +27,7 @@ jobs: - name: Build wheels run: | - pip install hatch==1.9.4 + pip install hatch==1.9.4 'click<8.3.0' # https://github.com/pallets/click/issues/3065 hatch build - name: Draft release From e0c8734f4f1b00779997f53dd19e6edb48412817 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Mon, 22 Sep 2025 21:20:34 +0530 Subject: [PATCH 05/10] added additional tests --- tests/unit/test_installation.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_installation.py b/tests/unit/test_installation.py index f475145..77e7610 100644 --- a/tests/unit/test_installation.py +++ b/tests/unit/test_installation.py @@ -771,9 +771,15 @@ class MissingAttributeClass: skip_validation: bool = True sdk_config: JsonValue = None - instance = MissingAttributeClass(False, {"warehouse_id": "8xc123456"}) + instance_false = MissingAttributeClass(False, {"warehouse_id": "8xc123456"}) installation = MockInstallation() - installation.save(instance) + installation.save(instance_false) - loaded = installation.load(MissingAttributeClass) - assert instance == loaded + loaded_false = installation.load(MissingAttributeClass) + assert instance_false == loaded_false + + instance_true = MissingAttributeClass(True, {"warehouse_id": "9xc1234567"}) + installation.save(instance_true) + + loaded_true = installation.load(MissingAttributeClass) + assert instance_true == loaded_true From 0727ad4306f83b9a9a6bc9fdd23a7a13128ca3c7 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Wed, 24 Sep 2025 20:02:50 +0530 Subject: [PATCH 06/10] modified the impact behavior to be just for bool type --- src/databricks/labs/blueprint/installation.py | 3 ++- tests/unit/test_installation.py | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/databricks/labs/blueprint/installation.py b/src/databricks/labs/blueprint/installation.py index 02412b3..e94951c 100644 --- a/src/databricks/labs/blueprint/installation.py +++ b/src/databricks/labs/blueprint/installation.py @@ -634,7 +634,8 @@ def _marshal_dataclass(self, type_ref: type, path: list[str], inst: Any) -> tupl raise SerdeError(self._explain_why(hint, [*path, field], raw)) # Earlier for boolean fields when the value was False, we would skip it. Which was incorrect if value is None: - continue + if hint is not type(bool): + continue as_dict[field] = value return as_dict, True diff --git a/tests/unit/test_installation.py b/tests/unit/test_installation.py index 77e7610..e2c5ad6 100644 --- a/tests/unit/test_installation.py +++ b/tests/unit/test_installation.py @@ -765,21 +765,21 @@ class ForwardReferencingClass: def test_bool_attribute() -> None: @dataclass - class MissingAttributeClass: + class BooleanAttributeClass: __file__ = "config.yml" __version__ = 3 skip_validation: bool = True sdk_config: JsonValue = None - instance_false = MissingAttributeClass(False, {"warehouse_id": "8xc123456"}) + instance_false = BooleanAttributeClass(False, {"warehouse_id": "8xc123456"}) installation = MockInstallation() installation.save(instance_false) - loaded_false = installation.load(MissingAttributeClass) + loaded_false = installation.load(BooleanAttributeClass) assert instance_false == loaded_false - instance_true = MissingAttributeClass(True, {"warehouse_id": "9xc1234567"}) + instance_true = BooleanAttributeClass(True, {"warehouse_id": "9xc1234567"}) installation.save(instance_true) - loaded_true = installation.load(MissingAttributeClass) + loaded_true = installation.load(BooleanAttributeClass) assert instance_true == loaded_true From b85264bcdde527998fa42468bd914a062117c832 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Wed, 24 Sep 2025 20:20:40 +0530 Subject: [PATCH 07/10] modified the impact behavior to be just for bool type --- src/databricks/labs/blueprint/installation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/databricks/labs/blueprint/installation.py b/src/databricks/labs/blueprint/installation.py index e94951c..a4bdada 100644 --- a/src/databricks/labs/blueprint/installation.py +++ b/src/databricks/labs/blueprint/installation.py @@ -633,8 +633,8 @@ def _marshal_dataclass(self, type_ref: type, path: list[str], inst: Any) -> tupl if not ok: raise SerdeError(self._explain_why(hint, [*path, field], raw)) # Earlier for boolean fields when the value was False, we would skip it. Which was incorrect - if value is None: - if hint is not type(bool): + if not value: + if hint is not bool: continue as_dict[field] = value return as_dict, True From b8fe090b9bacfada1b95e943e5e87cd800c77036 Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Wed, 24 Sep 2025 20:28:02 +0530 Subject: [PATCH 08/10] modified the impact behavior to be just for bool type --- src/databricks/labs/blueprint/installation.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/databricks/labs/blueprint/installation.py b/src/databricks/labs/blueprint/installation.py index a4bdada..93222f4 100644 --- a/src/databricks/labs/blueprint/installation.py +++ b/src/databricks/labs/blueprint/installation.py @@ -633,9 +633,8 @@ def _marshal_dataclass(self, type_ref: type, path: list[str], inst: Any) -> tupl if not ok: raise SerdeError(self._explain_why(hint, [*path, field], raw)) # Earlier for boolean fields when the value was False, we would skip it. Which was incorrect - if not value: - if hint is not bool: - continue + if not value and hint is not bool: + continue as_dict[field] = value return as_dict, True From f9329c95d062720d3459400202a4f37c2379a73e Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Wed, 24 Sep 2025 20:37:16 +0530 Subject: [PATCH 09/10] addressing review comments --- tests/unit/test_installation.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_installation.py b/tests/unit/test_installation.py index e2c5ad6..4f0cf89 100644 --- a/tests/unit/test_installation.py +++ b/tests/unit/test_installation.py @@ -764,21 +764,21 @@ class ForwardReferencingClass: def test_bool_attribute() -> None: - @dataclass + @dataclass(kw_only=True) class BooleanAttributeClass: __file__ = "config.yml" __version__ = 3 - skip_validation: bool = True - sdk_config: JsonValue = None + skip_validation: bool + sdk_config: JsonValue - instance_false = BooleanAttributeClass(False, {"warehouse_id": "8xc123456"}) + instance_false = BooleanAttributeClass(skip_validation=False, sdk_config={"warehouse_id": "8xc123456"}) installation = MockInstallation() installation.save(instance_false) loaded_false = installation.load(BooleanAttributeClass) assert instance_false == loaded_false - instance_true = BooleanAttributeClass(True, {"warehouse_id": "9xc1234567"}) + instance_true = BooleanAttributeClass(skip_validation=True, sdk_config={"warehouse_id": "9xc1234567"}) installation.save(instance_true) loaded_true = installation.load(BooleanAttributeClass) From fe57dfa2243615dae6d12c86c9becc3b0edd051e Mon Sep 17 00:00:00 2001 From: sundarshankar89 Date: Fri, 26 Sep 2025 10:56:38 +0530 Subject: [PATCH 10/10] fixed csv dumping logic --- src/databricks/labs/blueprint/installation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/databricks/labs/blueprint/installation.py b/src/databricks/labs/blueprint/installation.py index 93222f4..f4c3fbe 100644 --- a/src/databricks/labs/blueprint/installation.py +++ b/src/databricks/labs/blueprint/installation.py @@ -937,7 +937,7 @@ def _dump_csv(raw: list[JsonObject], type_ref: type) -> bytes: if not isinstance(as_dict, dict): raise SerdeError(f"Expecting a list of dictionaries. Got {as_dict}") for k, v in as_dict.items(): - if not v: + if not v and v is not False: continue non_empty_keys.add(k) buffer = io.StringIO()