Skip to content

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

Open
juliannguyen4 wants to merge 92 commits intodevfrom
CLIENT-3947-deprecate-meta-parameter-for-multiple-client-commands
Open

[CLIENT-3947] Deprecate meta["ttl"] parameter in favor of setting ttl as a policy option instead#880
juliannguyen4 wants to merge 92 commits intodevfrom
CLIENT-3947-deprecate-meta-parameter-for-multiple-client-commands

Conversation

@juliannguyen4
Copy link
Collaborator

@juliannguyen4 juliannguyen4 commented Dec 5, 2025

This affects all the aerospike.Client methods that accept a metadata dictionary parameter:

  • append()
  • increment()
  • operate()
  • operate_ordered()
  • prepend()
  • put()
  • remove_bin()
  • touch()

client.remove(): deprecate the meta parameter in favor of setting the generation in the remove policy parameter

As well as BatchRecords Write and Read.

Known Issues (out of scope of this PR)

  • If warnings are treated as errors, some of our API calls will raise ClientError instead of DeprecationWarning.
  • CLIENT-4334 test_cdt_index.py still uses the deprecated client.index_cdt_create()
  • Code coverage report does not reflect -Werror flag. Need to file separate ticket This is expected behavior
  • Clarify TTL_CLIENT_DEFAULT cannot be used for policy["ttl"]

Breaking Changes

  • client.touch(): when passing an invalid ttl to the policy parameter instead of meta parameter, ClientError will be raised instead of ParamError

Extra Changes

  • client.batch_remove(), batch_operate(), and batch_apply(): fix memory leak when a BatchRecord instance fails to be initialized
  • Start running client.remove() positive test cases
  • client.index_cdt_create(): if warnings are treated as errors, properly handle exception (TODO: needs verification)
  • validate_keys: allow "read_mode_ap", "max_threads", "thread_pool_size", and "socket_timeout" as valid keys in the client config dictionary's "policies" section
  • Fix test that checks if a valid client config option raises a ParamError TODO: needs better description
  • Doc: fix client.index_cdt_create() documentation being listed under aerospike module instead of aerospike.Client
  • Fix test_log.py fixture syntax
  • Fix test_remove.py positive tests being skipped and not properly being set up

Documentation

https://aerospike-python-client--880.org.readthedocs.build/en/880/client.html

TODO

  • Update code examples that use meta["ttl"] parameter

@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2025

Codecov Report

❌ Patch coverage is 57.14286% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.87%. Comparing base (b3a3aa4) to head (3efad5b).
⚠️ Report is 9 commits behind head on dev.

Files with missing lines Patch % Lines
src/main/client/batch_apply.c 0.00% 1 Missing ⚠️
src/main/client/batch_operate.c 0.00% 1 Missing ⚠️
src/main/client/batch_remove.c 0.00% 1 Missing ⚠️
src/main/client/remove.c 75.00% 1 Missing ⚠️
src/main/client/sec_index.c 66.66% 1 Missing ⚠️
src/main/conversions.c 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #880      +/-   ##
==========================================
+ Coverage   83.53%   83.87%   +0.33%     
==========================================
  Files          99       99              
  Lines       14392    14416      +24     
==========================================
+ Hits        12022    12091      +69     
+ Misses       2370     2325      -45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@juliannguyen4 juliannguyen4 force-pushed the CLIENT-3947-deprecate-meta-parameter-for-multiple-client-commands branch from 3efad5b to 09dbac0 Compare March 3, 2026 20:40
@juliannguyen4
Copy link
Collaborator Author

juliannguyen4 commented Mar 3, 2026

Build artifacts passes except for unrelated infra issues (Docker's dev branch now has the fix for macos arm64 runners failing) and Windows crash.

  • Currently testing whether Windows crash is related to PR or not by running those tests in dev

Massif memory usage looks ok

New memory leak in this PR still being reported. No memory errors reported:

2026-03-02T23:55:43.6704927Z ==6045== 64 bytes in 1 blocks are possibly lost in loss record 1,665 of 3,628
2026-03-02T23:55:43.6705149Z ==6045==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
2026-03-02T23:55:43.6705279Z ==6045==    by 0x4A34C52: _PyObject_GC_Alloc (gcmodule.c:2280)
2026-03-02T23:55:43.6705409Z ==6045==    by 0x4A34C52: _PyObject_GC_Malloc (gcmodule.c:2307)
2026-03-02T23:55:43.6705530Z ==6045==    by 0x4A34C52: _PyObject_GC_New (gcmodule.c:2319)
2026-03-02T23:55:43.6705640Z ==6045==    by 0x49CABEB: new_dict (dictobject.c:663)
2026-03-02T23:55:43.6705756Z ==6045==    by 0x49CABEB: PyDict_New (dictobject.c:745)
2026-03-02T23:55:43.6705889Z ==6045==    by 0x927DE85: bins_to_pyobject (conversions.c:2079)
2026-03-02T23:55:43.6706025Z ==6045==    by 0x927E125: record_to_pyobject (conversions.c:1911)
2026-03-02T23:55:43.6706198Z ==6045==    by 0x9280112: as_batch_result_to_BatchRecord (conversions.c:2726)
2026-03-02T23:55:43.6706326Z ==6045==    by 0x925E6D1: batch_apply_cb (batch_apply.c:80)
2026-03-02T23:55:43.6706487Z ==6045==    by 0x92B1D24: as_batch_keys_execute (aerospike_batch.c:3384)
2026-03-02T23:55:43.6706646Z ==6045==    by 0x92B5A21: aerospike_batch_apply (aerospike_batch.c:5323)
2026-03-02T23:55:43.6706834Z ==6045==    by 0x925EC39: AerospikeClient_Batch_Apply_Invoke (batch_apply.c:253)
2026-03-02T23:55:43.6707000Z ==6045==    by 0x925EF70: AerospikeClient_Batch_Apply (batch_apply.c:362)

Changes to test_remove.py aren't causing this memory leak.

@juliannguyen4
Copy link
Collaborator Author

juliannguyen4 commented Mar 6, 2026

Going to ignore "possibly lost" memory leaks from now on, since it seems the only valid leaks that valgrind detects from CPython are "definitely lost"

python/cpython#75675
Numpy valgrind guide

Did a thorough investigation on the leak and could not narrow down the test case causing the leak

@juliannguyen4 juliannguyen4 marked this pull request as ready for review March 6, 2026 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants