aerospike-client-python
aerospike-client-python copied to clipboard
[CLIENT-3121] Support multi-record transactions
Regression Testing
- [x] Massif memory usage looks ok
- [x] Build artifacts passes except for noise
- [x] Valgrind shows no memory errors from this PR
Extra changes:
CI/CD
- Always deploy enterprise edition server with strong consistency enabled along with security features
- Create separate folder for Dockerfile build context when deploying an enterprise edition server
- Allow running valgrind test on specific server version. This allows running against a server alpha version for MRTs when the latest tag for a server release candidate points to the next upcoming server release preceding MRTs
- Add option to run build wheels with specific test files for quick smoke testing
- Show exact test names that failed while tests are running during build wheels workflow
- Fix documentation spell check workflow failing
- Fix test node close listener for metrics
- Set default Docker image name for Dockerfile that deploys an EE server for testing
Misc
- Remove commented out list and map operations in type stubs that were removed in a prior release
Documentation changes: https://aerospike-python-client--674.org.readthedocs.build/en/674/client.html#multi-record-transactions https://aerospike-python-client--674.org.readthedocs.build/en/674/transaction.html https://aerospike-python-client--674.org.readthedocs.build/en/674/index.html
Extra changes: Cluster API breaking change: tran_count to command_count
todo
- [ ] Transaction type stub needs improvement
- [x] Check that new constants exist in tests... Don't need to test because type stubs test already checks that the module contains the constants
- [x] using c client code complete revision
- [x] using proper server config?
- [x] docs; explain what command and transaction mean
- [x] Dockerfile to deploy test server works with latest server release
Codecov Report
Attention: Patch coverage is 71.31783% with 37 lines in your changes missing coverage. Please review.
Project coverage is 80.79%. Comparing base (
8ff0327) to head (666a3c6). Report is 8 commits behind head on dev.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/main/client/mrt.c | 25.00% | 24 Missing :warning: |
| src/main/policy.c | 70.83% | 7 Missing :warning: |
| src/main/transaction/type.c | 91.42% | 6 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## dev #674 +/- ##
==========================================
- Coverage 80.89% 80.79% -0.10%
==========================================
Files 100 102 +2
Lines 15053 15173 +120
==========================================
+ Hits 12177 12259 +82
- Misses 2876 2914 +38
| Flag | Coverage Δ | |
|---|---|---|
? |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Looking good! I left a few nitpicks and have another thought. What do you think about making the mrt_commit_status_constants a class that subclasses enums.IntEnum? That way the user can tell what valid values are for those return codes through their IDE. This might not be easy to do and include in the base aerospike package since it is a C extension. If you have to define the class in C, I'd say it's not worth it. Let me know what you think.
Yeah it takes some effort to write an inheriting class in C; the C-API doesn't provide an API to create Enums. I was going to make a "dummy" package called aerospike and move the C-Python extension aerospike into that package, so we can declare types in Python using __init__.py, but I wasn't able to get it to work.