-
Notifications
You must be signed in to change notification settings - Fork 5
Client 3777 gh issue 63 get ttl behavior fix #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This patch introduces a PHP script in DEBUG-sfalvo where I am able to reproduce the OP's issue. To do this, I needed to make a small adjustment to the Rust binding code. After speaking with Khosrow, he recommended adding a few utility methods to the Expiration class ("We probably need a few methods in there, including neverExpire() -> bool, serverDefault() -> bool in addition to inSeconds() (in seconds should be an i64 that can be negative for the two above values.)). For now, this is just a research branch; DO NOT merge into main.
This patch greatly simplifies the getTtl() logic in the original PHP Rust bindings. It turns out that the Aerospike server itself maintains the TTL on behalf of the client; there is no need for the PHP client to maintain CITRUSLEAF_EPOCH-relative timestamps. This is in line with both the C and Java clients, thus implying consistent behavior with Javascript and Python clients as well. The Expiration interface has been upgraded as well; it is fully backward compatible with previous code, but now has getters which allows software to query its current state. In addition to this change, a number of PHPUnit tests have been added which exercises the new behavior along with the Expiration type's new interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we fix the pipeline failures?
a3e2032
to
941d6e1
Compare
ac415c3
to
dece2d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs an update to the aerospike-connection-manager/version.txt
and Cargo.toml
to v 1.4.0
Also introduces new test to repro and ensure this doesn't regress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not reviewing the auto-gend markup, but everything else look good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
No description provided.