Feat/kv store #4217
Feat/kv store #4217
Conversation
brian-mulier-p
force-pushed
the
feat/kv-store
branch
5 times, most recently
from
6e521dc
to
8f5dcff
Compare
brian-mulier-p
force-pushed
the
feat/kv-store
branch
4 times, most recently
from
20a47b9
to
8cdb6e8
Compare
brian-mulier-p
force-pushed
the
feat/kv-store
branch
from
8cdb6e8
to
ec1c2d4
Compare
brian-mulier-p
force-pushed
the
feat/kv-store
branch
from
ec1c2d4
to
50d12f4
Compare
|
| @@ -67,7 +71,12 @@ default boolean exists(String tenantId, URI uri) { | |||
| FileAttributes getAttributes(String tenantId, URI uri) throws IOException; | |||
|
|
|||
| @Retryable(includes = {IOException.class}) | |||
| URI put(String tenantId, URI uri, InputStream data) throws IOException; | |||
| default URI put(String tenantId, URI uri, InputStream data) throws IOException { | |||
There was a problem hiding this comment.
Would it be better to deprecate this one an always create the metadata? This can be handy for other functionalities.
There was a problem hiding this comment.
Idk since we never ever used metadata before maybe it's a better approach to leave it as-is and use the metadata one when there is a usecase for it ?
| @Parameter(description = "The key") @PathVariable String key, | ||
| @Body String value | ||
| ) throws IOException, URISyntaxException, ResourceExpiredException { | ||
| String ttl = httpHeaders.get("ttl"); |
There was a problem hiding this comment.
I would have add a parameter like for everything else as headers are usually for "technical stuff" and here TTL is a property of a KV.
There was a problem hiding this comment.
I don't really want to include the ttl in the body as it will be a pain to handle all the datatypes (I'll have to get everything as a String, check if that some json value, I'll probably lose some typing if I have a JSON wrapper everytime as in theory I then must have always a MediaType JSON then but then I was basing off that to know if I should convert to object then back to ion.
So a bit of technical constraint but aside of that I honestly don't see a true issue in ttl being in headers. I mean we store it in metadata in storage and I've already seen a lot of soft that are expecting such properties in headers 🤔
webserver/src/main/java/io/kestra/webserver/controllers/api/KVController.java
Show resolved
Hide resolved
core/src/main/java/io/kestra/core/runners/pebble/functions/KvFunction.java
Show resolved
Hide resolved
core/src/main/java/io/kestra/core/runners/pebble/functions/KvFunction.java
Show resolved
Hide resolved
| if (expirationDate != null && Instant.now().isAfter(expirationDate)) { | ||
| this.delete(key); | ||
|
|
||
| throw new ResourceExpiredException("The requested value has expired"); |
There was a problem hiding this comment.
I thought expiration is a natural behavior?
In this case I expect to return an empty optional (and maybe delete the kv).
There was a problem hiding this comment.
Idk I find it handy to know that the resource used to be there
part of #3609
closes #4232