-
Notifications
You must be signed in to change notification settings - Fork 192
DATACOUCH-608 - propagate expiry option and annotation to insert and … #259
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
DATACOUCH-608 - propagate expiry option and annotation to insert and … #259
Conversation
ff06fb5
to
c846ec4
Compare
|
||
@Override | ||
public InsertByIdWithDurability<T> withExpiry(final Duration expiry) { | ||
return new ExecutableInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, |
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.
Assert.notNull(expiry,..) checking might make sense here
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.
Added.
|
||
@Override | ||
public UpsertByIdWithDurability<T> withExpiry(final Duration expiry) { | ||
Assert.notNull(persistTo, "PersistTo must not be null."); |
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.
these assertions are checking the wrong thing, copy/paste err?
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.
Yes. Replaced with correct check on expiry. Also changed initialization to use expiry of Duration.ofSeconds(0) instead of null for consistency.
try { | ||
User inserted = couchbaseTemplate.insertById(User.class).withExpiry(Duration.ofSeconds(1)).one(user); | ||
assertEquals(user, inserted); | ||
sleepSecs(2); |
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.
are these sleeps needed in those places? kv should not be eventually consistent but rather directly
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.
The sleeps are need to allow the document to expire.
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.
of course 🤦
c846ec4
to
bd0252a
Compare
…upsert calls