Skip to content

fix: replace deprecated JSONPropertiesContainer with AbstractPropContainer - #2101

Open
nanookclaw wants to merge 2 commits into
NVIDIA:mainfrom
nanookclaw:fix/deprecated-json-properties-container
Open

nanookclaw wants to merge 2 commits into
NVIDIA:mainfrom
nanookclaw:fix/deprecated-json-properties-container

Conversation

@nanookclaw

Copy link
Copy Markdown

Replaces deprecated JSONPropertiesContainer with AbstractPropContainer in two files, per the deprecation warnings on AbstractPropertiesContainer, YAMLPropertiesContainer, and JSONPropertiesContainer in prop_manager.py.

Changes

dataproc.py (7 call sites):

  • Import: AbstractPropContainer from spark_rapids_tools.utils.propmanager; kept is_valid_gpu_device from prop_manager
  • JSONPropertiesContainer(prop_arg=X, file_load=False)AbstractPropContainer(props=X) at 5 locations (pricing config, instance descriptions, worker props, master props)
  • Type annotation updates at pricing_config parameter and _set_zone_from_props signature

cluster_inference.py (1 call site):

  • Import swap to AbstractPropContainer
  • JSONPropertiesContainer(cluster_conf, file_load=False)AbstractPropContainer(props=cluster_conf)

All changes follow the AbstractPropContainer API: props= for dict wrapping, matching get_value/get_value_silent method signatures preserved from the parent class.

Closes #1898

@github-actions github-actions Bot added user_tools Scope the wrapper module running CSP, QualX, and reports (python) core_tools Scope the core module (scala) labels Jun 8, 2026
…ainer

Replace deprecated JSONPropertiesContainer usage with AbstractPropContainer
in dataproc.py and cluster_inference.py per deprecation warnings.

Closes NVIDIA#1898

Signed-off-by: Nanook <nanookclaw@users.noreply.github.com>
@nanookclaw
nanookclaw force-pushed the fix/deprecated-json-properties-container branch from 16a6aec to 79b0d19 Compare June 8, 2026 13:15
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously reported JSON-boundary regressions are fixed and no new actionable issue remains.

Summary

  • Dataproc instance descriptions accept either serialized JSON or decoded lists.
  • Cluster inference accepts either rendered JSON text or decoded dictionaries.
  • Regression tests cover both representations at each corrected boundary.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[CLI or rendered template output] --> B{Value is a string?}
    B -->|Yes| C[json.loads]
    B -->|No| D[Use decoded value]
    C --> E[AbstractPropContainer]
    D --> E
    E --> F[Dataproc processing or cluster loading]
Loading

Reviews (2) · Last reviewed commit: "fix: decode serialized JSON configuratio..."

Comment thread user_tools/src/spark_rapids_pytools/cloud_api/dataproc.py
Comment thread user_tools/src/spark_rapids_pytools/common/cluster_inference.py
Comment thread user_tools/src/spark_rapids_pytools/cloud_api/dataproc.py
Comment thread user_tools/src/spark_rapids_pytools/common/cluster_inference.py
Signed-off-by: Nanook <nanookclaw@users.noreply.github.com>
@nanookclaw

Copy link
Copy Markdown
Author

Updated at commit cafb5be586a8dbae862f2ced016f4a54fa2c415e.

The change decodes serialized JSON at the Dataproc instance-description and cluster-inference configuration boundaries while preserving already-decoded inputs. Added focused regression coverage for both forms at each boundary. The Dataproc test exercises DataprocCMDDriver._process_instance_description.

Local verification under Python 3.12.3: focused test_json_boundaries.py — 4 passed; py_compile — passed; git diff --check — passed. The commit includes the required DCO sign-off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core_tools Scope the core module (scala) user_tools Scope the wrapper module running CSP, QualX, and reports (python)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants