From: Brett Smith <brett.smith@curii.com>
Date: Fri, 29 Sep 2023 01:35:38 +0000 (-0400)
Subject: 19818: Deprecate arvados.api.OrderedJsonModel
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/df893c906a6387ade4767425ad8c68e2289ca359

19818: Deprecate arvados.api.OrderedJsonModel

Python dictionaries started preserving insertion order in CPython 3.6,
and that became a specified feature of the language in Python 3.7. That
means the default model has the same behavior as OrderedJsonModel on
every platform supported by Arvados. Deprecate the class accordingly.

Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith@curii.com>
---

diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py
index 8108934aae..7968fb1e2b 100644
--- a/sdk/cwl/arvados_cwl/__init__.py
+++ b/sdk/cwl/arvados_cwl/__init__.py
@@ -32,7 +32,6 @@ import arvados.logging
 from arvados.keep import KeepClient
 from arvados.errors import ApiError
 import arvados.commands._util as arv_cmd
-from arvados.api import OrderedJsonModel
 
 from .perf import Perf
 from ._version import __version__
@@ -338,7 +337,6 @@ def main(args=sys.argv[1:],
         if api_client is None:
             api_client = arvados.safeapi.ThreadSafeApiCache(
                 api_params={
-                    'model': OrderedJsonModel(),
                     'num_retries': arvargs.retries,
                     'timeout': arvargs.http_timeout,
                 },
diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index 94b8013997..d2a705b903 100644
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -61,21 +61,20 @@ if sys.version_info >= (3,):
 class OrderedJsonModel(apiclient.model.JsonModel):
     """Model class for JSON that preserves the contents' order.
 
-    API clients that care about preserving the order of fields in API
-    server responses can use this model to do so, like this:
+    .. WARNING:: Deprecated
+       This model is redundant now that Python dictionaries preserve insertion
+       ordering. Code that passes this model to API constructors can remove it.
+
+    In Python versions before 3.6, API clients that cared about preserving the
+    order of fields in API server responses could use this model to do so.
+    Typical usage looked like:
 
         from arvados.api import OrderedJsonModel
         client = arvados.api('v1', ..., model=OrderedJsonModel())
     """
-
-    def deserialize(self, content):
-        # This is a very slightly modified version of the parent class'
-        # implementation.  Copyright (c) 2010 Google.
-        content = content.decode('utf-8')
-        body = json.loads(content, object_pairs_hook=collections.OrderedDict)
-        if self._data_wrapper and isinstance(body, dict) and 'data' in body:
-            body = body['data']
-        return body
+    @util._deprecated(preferred="the default model and rely on Python's built-in dictionary ordering")
+    def __init__(self, data_wrapper=False):
+        return super().__init__(data_wrapper)
 
 
 _orig_retry_request = apiclient.http._retry_request
diff --git a/sdk/python/arvados/commands/arv_copy.py b/sdk/python/arvados/commands/arv_copy.py
index 10fe9d7024..41b500a52f 100755
--- a/sdk/python/arvados/commands/arv_copy.py
+++ b/sdk/python/arvados/commands/arv_copy.py
@@ -45,7 +45,6 @@ import arvados.commands._util as arv_cmd
 import arvados.commands.keepdocker
 import ruamel.yaml as yaml
 
-from arvados.api import OrderedJsonModel
 from arvados._version import __version__
 
 COMMIT_HASH_RE = re.compile(r'^[0-9a-f]{1,40}$')
@@ -191,7 +190,7 @@ def set_src_owner_uuid(resource, uuid, args):
 def api_for_instance(instance_name, num_retries):
     if not instance_name:
         # Use environment
-        return arvados.api('v1', model=OrderedJsonModel())
+        return arvados.api('v1')
 
     if '/' in instance_name:
         config_file = instance_name
@@ -215,7 +214,6 @@ def api_for_instance(instance_name, num_retries):
                              host=cfg['ARVADOS_API_HOST'],
                              token=cfg['ARVADOS_API_TOKEN'],
                              insecure=api_is_insecure,
-                             model=OrderedJsonModel(),
                              num_retries=num_retries,
                              )
     else:
diff --git a/tools/crunchstat-summary/crunchstat_summary/summarizer.py b/tools/crunchstat-summary/crunchstat_summary/summarizer.py
index a876257abc..2bd329719b 100644
--- a/tools/crunchstat-summary/crunchstat_summary/summarizer.py
+++ b/tools/crunchstat-summary/crunchstat_summary/summarizer.py
@@ -15,7 +15,6 @@ import sys
 import threading
 import _strptime
 
-from arvados.api import OrderedJsonModel
 from crunchstat_summary import logger
 
 # Recommend memory constraints that are this multiple of an integral
@@ -518,7 +517,7 @@ def NewSummarizer(process_or_uuid, **kwargs):
     else:
         uuid = process_or_uuid
         process = None
-        arv = arvados.api('v1', model=OrderedJsonModel())
+        arv = arvados.api('v1')
 
     if '-dz642-' in uuid:
         if process is None:
@@ -639,7 +638,7 @@ class MultiSummarizer(object):
 class JobTreeSummarizer(MultiSummarizer):
     """Summarizes a job and all children listed in its components field."""
     def __init__(self, job, label=None, **kwargs):
-        arv = arvados.api('v1', model=OrderedJsonModel())
+        arv = arvados.api('v1')
         label = label or job.get('name', job['uuid'])
         children = collections.OrderedDict()
         children[job['uuid']] = JobSummarizer(job, label=label, **kwargs)
@@ -683,7 +682,7 @@ class PipelineSummarizer(MultiSummarizer):
 
 class ContainerRequestTreeSummarizer(MultiSummarizer):
     def __init__(self, root, skip_child_jobs=False, **kwargs):
-        arv = arvados.api('v1', model=OrderedJsonModel())
+        arv = arvados.api('v1')
 
         label = kwargs.pop('label', None) or root.get('name') or root['uuid']
         root['name'] = label