[CLIENT-3947] Deprecate meta["ttl"] parameter in favor of setting ttl as a policy option instead

.github/workflows/smoke-tests.yml

@@ -136,7 +136,7 @@ jobs:
         registry-username: ${{ env.REGISTRY_NAME == 'docker.io' && secrets.DOCKER_HUB_BOT_USERNAME || secrets.QE_DOCKER_REGISTRY_USERNAME }}
         registry-password: ${{ env.REGISTRY_NAME == 'docker.io' && secrets.DOCKER_HUB_BOT_PW || secrets.QE_DOCKER_REGISTRY_PASSWORD }}
 
-    - run: python3 -m pytest --cov=aerospike_helpers --cov-report xml:coverage.xml ./new_tests
+    - run: python3 -m pytest -Werror --cov=aerospike_helpers --cov-report xml:coverage.xml ./new_tests
       working-directory: test
 
     - name: Copy over source files to build dir
@@ -306,7 +306,7 @@ jobs:
     - name: Run tests
       # We need to disable capturing output or else we cannot see memory errors reported by
       # libasan during the test run
-      run: python -m pytest ./new_tests -vvs
+      run: python -m pytest ./new_tests -vvs -W error::DeprecationWarning
       working-directory: test
 
   test-ce:

aerospike_helpers/batch/records.py

@@ -60,6 +60,8 @@ def __init__(self, key: tuple) -> None:
 class Write(BatchRecord):
     """ Write is used for executing Batch write commands with batch_write and retrieving batch write results.
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         Attributes:
             key (:obj:`tuple`): The aerospike key to send the command to.
             record (:obj:`tuple`): The record corresponding to the requested key.
@@ -111,6 +113,9 @@ def __init__(
 class Read(BatchRecord):
     """ Read is used for executing Batch read commands with batch_write and retrieving results.
 
+        .. deprecated:: 19.1.0 Deprecated the ``"ttl"`` option in the ``meta`` parameter. Use the policy parameter in a
+            :py:obj:`~aerospike_helpers.batch.records.Write` BatchRecord to set the ``"ttl"`` instead.
+
         Attributes:
             key (:obj:`tuple`): The aerospike key to send the command to.
             record (:obj:`tuple`): The record corresponding to the requested key.

aerospike_helpers/operations/operations.py

@@ -119,18 +119,18 @@ def increment(bin_name, amount):
 def touch(ttl: Optional[int] = None):
     """Create a touch operation dictionary.
 
-    Using ttl here is deprecated. It should be set in the record metadata for the operate method.
+    Using ttl here is deprecated. It should be set in the policy parameter for the operate method.
 
     Args:
         ttl (int): Deprecated. The ttl that should be set for the record.
-            This should be set in the metadata passed to the operate or
+            This should be set in the policy parameter passed to the operate or
             operate_ordered methods.
     Returns:
         A dictionary to be passed to operate or operate_ordered.
     """
     op_dict = {"op": aerospike.OPERATOR_TOUCH}
     if ttl:
-        warnings.warn("TTL should be specified in the meta dictionary for operate", DeprecationWarning)
+        warnings.warn("TTL should be specified in the policy parameter for operate", DeprecationWarning)
         op_dict["val"] = ttl
     return op_dict
 

doc/client.rst

@@ -100,6 +100,8 @@ Record Commands
 
         Create a new record, or remove / add bins to a record.
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         :param tuple key: a :ref:`aerospike_key_tuple` associated with the record.
         :param dict bins: contains bin name-value pairs of the record.
         :param dict meta: record metadata to be set. see :ref:`metadata_dict`.
@@ -181,6 +183,8 @@ Record Commands
         (In Aerospike server versions prior to 3.6.0, non-existent bins being read will have a \
         :py:obj:`None` value. )
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         :param tuple key: a :ref:`aerospike_key_tuple` associated with the record.
         :param list list: See :ref:`aerospike_operation_helpers.operations`.
         :param dict meta: record metadata to be set. See :ref:`metadata_dict`.
@@ -207,6 +211,8 @@ Record Commands
 
         Write operations or read operations that fail will not return a ``(bin-name, result)`` tuple.
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         :param tuple key: a :ref:`aerospike_key_tuple` associated with the record.
         :param list list: See :ref:`aerospike_operation_helpers.operations`.
         :param dict meta: record metadata to be set. See :ref:`metadata_dict`.
@@ -224,6 +230,10 @@ Record Commands
 
         Touch the given record, setting its time-to-live and incrementing its generation.
 
+        .. versionchanged:: 19.1.0
+
+            Deprecated the ``meta["ttl"]`` parameter. Use the ``val`` parameter instead.
+
         :param tuple key: a :ref:`aerospike_key_tuple` associated with the record.
         :param int val: ttl in seconds, with ``0`` resolving to the default value in the server config.
         :param dict meta: record metadata to be set. see :ref:`metadata_dict`
@@ -238,6 +248,10 @@ Record Commands
 
         Remove a record matching the *key* from the cluster.
 
+        .. versionchanged:: 19.1.0
+
+            Deprecated the ``meta`` parameter. Use the policy parameter to set ``gen`` instead.
+
         :param tuple key: a :ref:`aerospike_key_tuple` associated with the record.
         :param dict meta: contains the expected generation of the record in a key called ``"gen"``.
         :param dict policy: see :ref:`aerospike_remove_policies`. May be passed as a keyword argument.
@@ -252,6 +266,8 @@ Record Commands
         Remove a list of bins from a record with a given *key*. Equivalent to \
         setting those bins to :meth:`aerospike.null` with a :meth:`~aerospike.put`.
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         :param tuple key: a :ref:`aerospike_key_tuple` associated with the record.
         :param list list: the bins names to be removed from the record.
         :param dict meta: record metadata to be set. See :ref:`metadata_dict`.
@@ -408,6 +424,8 @@ String Operations
 
         Append a string to the string value in bin.
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         :param tuple key: a :ref:`aerospike_key_tuple` tuple associated with the record.
         :param str bin: the name of the bin.
         :param str val: the string to append to the bin value.
@@ -427,6 +445,8 @@ String Operations
 
         Prepend the string value in *bin* with the string *val*.
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         :param tuple key: a :ref:`aerospike_key_tuple` tuple associated with the record.
         :param str bin: the name of the bin.
         :param str val: the string to prepend to the bin value.
@@ -456,6 +476,8 @@ Numeric Operations
 
         Increment the integer value in *bin* by the integer *val*.
 
+        .. include:: ./deprecate_meta_ttl.rst
+
         :param tuple key: a :ref:`aerospike_key_tuple` tuple associated with the record.
         :param str bin: the name of the bin.
         :param int offset: the value by which to increment the value in *bin*.
@@ -946,7 +968,7 @@ Index Operations
 
     .. method:: index_string_create(ns, set, bin, name[, policy: dict])
 
-        .. deprecated:: 20.0.0 :meth:`index_single_value_create` should be used instead.
+        .. deprecated:: 19.1.0 :meth:`index_single_value_create` should be used instead.
 
         Create a string index with *index_name* on the *bin* in the specified \
         *ns*, *set*.
@@ -960,7 +982,7 @@ Index Operations
 
     .. method:: index_integer_create(ns, set, bin, name[, policy])
 
-        .. deprecated:: 20.0.0 :meth:`index_single_value_create` should be used instead.
+        .. deprecated:: 19.1.0 :meth:`index_single_value_create` should be used instead.
 
         Create an integer index with *name* on the *bin* in the specified \
         *ns*, *set*.
@@ -974,7 +996,7 @@ Index Operations
 
     .. method:: index_blob_create(ns, set, bin, name[, policy])
 
-        .. deprecated:: 20.0.0 :meth:`index_single_value_create` should be used instead.
+        .. deprecated:: 19.1.0 :meth:`index_single_value_create` should be used instead.
 
         Create a blob index with *name* on the *bin* in the specified \
         *ns*, *set*.
@@ -988,7 +1010,7 @@ Index Operations
 
     .. method:: index_geo2dsphere_create(ns, set, bin, name[, policy: dict])
 
-        .. deprecated:: 20.0.0 :meth:`index_single_value_create` should be used instead.
+        .. deprecated:: 19.1.0 :meth:`index_single_value_create` should be used instead.
 
         Create a geospatial 2D spherical index with *name* on the *bin* \
         in the specified *ns*, *set*.
@@ -1012,23 +1034,23 @@ Index Operations
             client.index_geo2dsphere_create('test', 'pads', 'loc', 'pads_loc_geo')
             client.close()
 
-.. method:: index_cdt_create(ns: str, set: str, bin: str, index_type, index_datatype, index_name: str, ctx: list[, policy: dict])
+    .. method:: index_cdt_create(ns: str, set: str, bin: str, index_type, index_datatype, index_name: str, ctx: list[, policy: dict])
 
-    .. deprecated:: 20.0.0 Use the other non-deprecated index methods to create an index with a list of contexts.
+        .. deprecated:: 19.1.0 Use the other non-deprecated index methods to create an index with a list of contexts.
 
-    Create an collection data type (CDT) index named *index_name* for a scalar, list values, map keys, or map values (as defined by *index_type*) and for
-    numeric, string, or GeoJSON values (as defined by *index_datatype*)
-    on records of the specified *ns*, *set* whose bin is a list or map.
+        Create an collection data type (CDT) index named *index_name* for a scalar, list values, map keys, or map values (as defined by *index_type*) and for
+        numeric, string, or GeoJSON values (as defined by *index_datatype*)
+        on records of the specified *ns*, *set* whose bin is a list or map.
 
-    :param str ns: the namespace in the aerospike cluster.
-    :param str set: the set name.
-    :param str bin: the name of bin the secondary index is built on.
-    :param index_type: whether we are querying a single scalar value or specific values of a CDT type. See :ref:`aerospike_index_types`.
-    :param index_datatype: the type of value being queried on. See :ref:`aerospike_index_datatypes`.
-    :param str index_name: the name of the index.
-    :param dict ctx: a :class:`list` of contexts produced by :mod:`aerospike_helpers.cdt_ctx` methods.
-    :param dict policy: optional :ref:`aerospike_info_policies`.
-    :raises: a subclass of :exc:`~aerospike.exception.AerospikeError`.
+        :param str ns: the namespace in the aerospike cluster.
+        :param str set: the set name.
+        :param str bin: the name of bin the secondary index is built on.
+        :param index_type: whether we are querying a single scalar value or specific values of a CDT type. See :ref:`aerospike_index_types`.
+        :param index_datatype: the type of value being queried on. See :ref:`aerospike_index_datatypes`.
+        :param str index_name: the name of the index.
+        :param dict ctx: a :class:`list` of contexts produced by :mod:`aerospike_helpers.cdt_ctx` methods.
+        :param dict policy: optional :ref:`aerospike_info_policies`.
+        :raises: a subclass of :exc:`~aerospike.exception.AerospikeError`.
 
     .. index::
         single: Admin Operations

doc/deprecate_meta_ttl.rst

@@ -0,0 +1,3 @@
+.. versionchanged:: 19.1.0
+
+    Deprecated the ``meta["ttl"]`` parameter. Use the policy parameter to set ``ttl`` instead.

src/include/client.h

@@ -25,6 +25,9 @@
 
 #define CLUSTER_NPARTITIONS (4096)
 
+// This allows people to see the function calling the Python client API that issues a warning
+#define STACK_LEVEL 2
+
 /*******************************************************************************
  * Macros for UDF operations.
  ******************************************************************************/

src/main/aerospike.c

@@ -661,7 +661,9 @@ DEFINE_SET_OF_VALID_KEYS(client_config_policies, "read", "write", "apply",
                          "batch_parent_write", "info", "admin", "txn_verify",
                          "txn_roll", "total_timeout", "auth_mode",
                          "login_timeout_ms", "key", "exists", "max_retries",
-                         "replica", "commit_level", "metrics", NULL)
+                         "replica", "commit_level", "metrics", "read_mode_ap",
+                         "max_threads", "thread_pool_size", "socket_timeout",
+                         NULL)
 
 DEFINE_SET_OF_VALID_KEYS(client_config_tls, "enable", "cafile", "capath",
                          "protocols", "cipher_suite", "keyfile", "keyfile_pw",

src/main/client/remove.c

@@ -169,6 +169,17 @@ PyObject *AerospikeClient_Remove(AerospikeClient *self, PyObject *args,
         return NULL;
     }
 
+    if (py_meta) {
+        int retval = PyErr_WarnEx(
+            PyExc_DeprecationWarning,
+            "meta parameter is deprecated and will be removed in the "
+            "next client major release",
+            STACK_LEVEL);
+        if (retval == -1) {
+            return NULL;
+        }
+    }
+
     // Invoke Operation
     return AerospikeClient_Remove_Invoke(self, py_key, py_meta, py_policy);
 }

src/main/client/sec_index.c

@@ -241,9 +241,6 @@ PyObject *AerospikeClient_Index_Expr_Create(AerospikeClient *self,
         NULL, py_expr);
 }
 
-// This allows people to see the function calling the Python client API that issues a warning
-#define STACK_LEVEL 2
-
 // TODO: way to get method name dynamically for error message?
 static inline PyObject *
 AerospikeClient_Index_Create_Helper(AerospikeClient *self, PyObject *args,
@@ -469,10 +466,14 @@ PyObject *AerospikeClient_Index_2dsphere_Create(AerospikeClient *self,
 PyObject *AerospikeClient_Index_Cdt_Create(AerospikeClient *self,
                                            PyObject *args, PyObject *kwds)
 {
-    PyErr_WarnEx(PyExc_DeprecationWarning,
-                 "index_cdt_create() is deprecated. Please use one of the "
-                 "other non-deprecated index_*_create() methods instead",
-                 STACK_LEVEL);
+    int retval =
+        PyErr_WarnEx(PyExc_DeprecationWarning,
+                     "index_cdt_create() is deprecated. Please use one of the "
+                     "other non-deprecated index_*_create() methods instead",
+                     STACK_LEVEL);
+    if (retval == -1) {
+        return NULL;
+    }
 
     // Initialize error
     as_error err;

src/main/conversions.c

@@ -2289,6 +2289,10 @@ void initialize_bin_for_strictypes(AerospikeClient *self, as_error *err,
     strcpy(binop_bin->name, bin);
 }
 
+#define META_TTL_DEPRECATION_MESSAGE                                           \
+    "meta[\"ttl\"] is deprecated and will be removed in "                      \
+    "the next client major release."
+
 /**
  *******************************************************************************************************
  * This function checks for metadata and if present set it into the
@@ -2327,6 +2331,16 @@ as_status check_and_set_meta(PyObject *py_meta, uint32_t *ttl_ref,
         uint32_t ttl = 0;
         uint16_t gen = 0;
         if (py_ttl) {
+            int retval =
+                PyErr_WarnEx(PyExc_DeprecationWarning,
+                             META_TTL_DEPRECATION_MESSAGE, STACK_LEVEL);
+            if (retval == -1) {
+                // This handles the codepath where warnings are converted into errors from pytest/python cli
+                // TODO: this does NOT handle the codepath where the warning mechanism itself fails
+                return as_error_update(err, AEROSPIKE_ERR,
+                                       META_TTL_DEPRECATION_MESSAGE);
+            }
+
             if (PyLong_Check(py_ttl)) {
                 ttl = (uint32_t)PyLong_AsLong(py_ttl);
             }

test/new_tests/test_append.py

@@ -113,9 +113,10 @@ def test_pos_append_with_policy_key_gen_EQ_ignore(self):
         key = ("test", "demo", 1)
         policy = {
             "gen": aerospike.POLICY_GEN_IGNORE,
+            "ttl": 1200
         }
 
-        meta = {"gen": 10, "ttl": 1200}
+        meta = {"gen": 10}
         self.as_connection.append(key, "name", "str", meta, policy)
 
         (key, meta, bins) = self.as_connection.get(key)
@@ -135,12 +136,13 @@ def test_pos_append_with_policy_key_gen_EQ_positive(self):
         key = ("test", "demo", 1)
         policy = {
             "gen": aerospike.POLICY_GEN_EQ,
+            "ttl": 1200
         }
         (key, meta) = self.as_connection.exists(key)
 
         gen = meta["gen"]
 
-        meta = {"gen": gen, "ttl": 1200}
+        meta = {"gen": gen}
         self.as_connection.append(key, "name", "str", meta, policy)
 
         (key, meta, bins) = self.as_connection.get(key)
@@ -160,11 +162,12 @@ def test_pos_append_with_policy_key_gen_GT_positive(self):
         key = ("test", "demo", 1)
         policy = {
             "gen": aerospike.POLICY_GEN_GT,
+            "ttl": 1200
         }
         (key, meta) = self.as_connection.exists(key)
         gen = meta["gen"]
 
-        meta = {"gen": gen + 2, "ttl": 1200}
+        meta = {"gen": gen + 2}
         self.as_connection.append(key, "name", "str", meta, policy)
 
         (key, meta, bins) = self.as_connection.get(key)
@@ -294,12 +297,13 @@ def test_neg_append_with_policy_key_gen_GT_lesser(self):
         key = ("test", "demo", 1)
         policy = {
             "gen": aerospike.POLICY_GEN_GT,
+            "ttl": 1200
         }
         (key, meta) = self.as_connection.exists(key)
 
         gen = meta["gen"]
 
-        meta = {"gen": gen, "ttl": 1200}
+        meta = {"gen": gen}
         try:
             self.as_connection.append(key, "name", "str", meta, policy)
         except e.RecordGenerationError as exception:
@@ -321,11 +325,12 @@ def test_neg_append_with_policy_key_gen_EQ_not_equal(self):
         key = ("test", "demo", 1)
         policy = {
             "gen": aerospike.POLICY_GEN_EQ,
+            "ttl": 1200
         }
         (key, meta) = self.as_connection.exists(key)
         gen = meta["gen"]
 
-        meta = {"gen": gen + 5, "ttl": 1200}
+        meta = {"gen": gen + 5}
         try:
             self.as_connection.append(key, "name", "str", meta, policy)