search

aerospike / aerospike-client-python

Aerospike Python Client
Apache License 2.0
133 stars 111 forks source link

[CLIENT-3106] Refactor aerospike.exception initialization code and check if error indicator is set after calling a C-API method #661

Closed juliannguyen4 closed 1 month ago

juliannguyen4 commented 2 months ago

Documentation changes:

https://aerospike-python-client--661.org.readthedocs.build/en/661/exception.html#aerospike.exception.QueryAbortedError https://aerospike-python-client--661.org.readthedocs.build/en/661/exception.html#aerospike.exception.QueryError https://aerospike-python-client--661.org.readthedocs.build/en/661/exception.html#aerospike.exception.ClusterError https://aerospike-python-client--661.org.readthedocs.build/en/661/exception.html#aerospike.exception.AdminError

Extra changes:

Notes: Calling import aerospike or from aerospike import exception multiple times is idempotent. Once a module is loaded, it cannot be loaded again just by calling import on it (verified this through gdb by adding a breakpoint at AerospikeException_New. It is only hit once during the tests)

All artifacts pass testing: https://github.com/aerospike/aerospike-client-python/actions/runs/10776301993

No memory errors: https://github.com/aerospike/aerospike-client-python/actions/runs/10775987465/job/29881798616

Performance regression testing:

Memory

Dev (baseline): https://github.com/aerospike/aerospike-client-python/actions/runs/10708250636/job/29690214082#step:13:20 Feature branch: https://github.com/aerospike/aerospike-client-python/actions/runs/10775988854/job/29881848296#step:13:20

Time performance:

# 15.0.1rc5.dev2
(.venv) jnguyen@AerospikesMBP4 aerospike-client-python % python3 -m timeit -n 50000 "from aerospike import exception"
50000 loops, best of 5: 120 nsec per loop
# this branch
(.venv) jnguyen@AerospikesMBP4 aerospike-client-python % python3 -m timeit -n 50000 "from aerospike import exception"
50000 loops, best of 5: 121 nsec per loop

Doesn't seem to be much of a difference

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 67.92453% with 17 lines in your changes missing coverage. Please review.

Project coverage is 80.96%. Comparing base (1ffc37d) to head (13227ed). Report is 6 commits behind head on dev.

Files with missing lines Patch % Lines
src/main/exception.c 67.30% 17 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #661 +/- ## ========================================== - Coverage 81.33% 80.96% -0.38% ========================================== Files 100 100 Lines 15346 15130 -216 ========================================== - Hits 12482 12250 -232 - Misses 2864 2880 +16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.