Add support for NATS KV Processor/Input/Output. Add default NATS urls #1704
Conversation
|
FYI on failing builds |
|
Great work! Would it be worth upgrading to the latest version of the NATS client as part of this pull request? I see Benthos is currently using a version released in May 2022. |
| metaKVBucket, keyValue.Bucket(), | ||
| metaKVKey, key, | ||
| metaKVRevision, rev, | ||
| ).Debug("Updated kv bucket entry") |
There was a problem hiding this comment.
A minor one but I see in other log entries "kv" is uppercase. Perhaps it should be capitalised here too for consistency.
Oh yeah. Great reminder. I'll go ahead and upgrade the client |
Thems is fighting words, I'm gonna review this one on stream 😎 |
…h is not working properly
Love it, but I would maybe suggest cache would be a better first addition, as there are output and processor implementations around cache resources that are composable, which is pretty awesome. iirc there is not a pattern to input from a cache so that use case would have to be covered by a new input type, but it seems like the processor and output may be better to utilize what is there. Just my 2 cents! |
| @@ -33,6 +33,7 @@ You can access these metadata fields using [function interpolation](/docs/config | |||
| ` + auth.Description()). | |||
| Field(service.NewStringListField("urls"). | |||
| Description("A list of URLs to connect to. If an item of the list contains commas it will be expanded into multiple URLs."). | |||
| Default([]string{"nats://127.0.0.1:4222"}). | |||
There was a problem hiding this comment.
Generally I avoid having defaults for stuff like urls and addresses, we used to have them all over the shop and people kept getting bitten when forgetting to specify them explicitly.
| return nil, nil, service.ErrNotConnected | ||
| } | ||
|
|
||
| if entry == nil { |
There was a problem hiding this comment.
I'm assuming this could happen if some internal timeout occurs? Just making sure we can't enter a busy loop here.
| # We need to make this key random as the NATS server will only deliver the | ||
| # latest revision of a key when it's requested by a watcher, this is by | ||
| # design, but if we want to test benthos semantics like batching we should | ||
| # use unique keys for ever message passing through the output |
There was a problem hiding this comment.
ERRRRRMMMMMMMM... I think you mean every message...
What a disaster 😓
This PR goes out to all the blobs out there who believed there would come a day where Benthos would have more NATS support:
puts,gets,deletes,creates,updates,deletes, slices, dices and even folds your laundry (if you are so inclined)We got docs! 😮
We got integration tests! 😄
We even got default urls for NATS components! 🤷 (for real though, We like using the default URL in all our NATS tool, since NATS always kicks itself off on
localhost:4222, I think this makes it a wee bit easier to get started with NATS and Benthos)Go ahead, tear it apart.... I dare you.
Bring it el jeffe! (aka @Jeffail )
And if you are really nice I'll write a Cache next