feat: Make watermill settings configurable #1297
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JAORMX
changed the title
evankanderson
previously approved these changes
Oct 26, 2023
|
I suspect we might want to start with a lower value... do we want to wire this in with the viper configuration? |
|
I'm gonna make this configurable. Let it as WIP until I add it |
JAORMX
force-pushed
the
parallel-watermill
branch
from
beb29f1
to
acfd59f
Compare
JAORMX
changed the title
This adds configuration options for our watermill implementation. For now we only have a `go-channel` driver, which is what we've been using until now. This is also the default. The first configuration settings this adds for the aforementioned driver as the following: * `router_close_timeout`: per the watermill docs this determines how long router should work for handlers when closing. * `buffer_size`: determines the buffer size for the golang channel the driver is using. By allowing for setting the buffer size, we allow for paralellization of request handling of events. We were not setting this before and thus our event handling was serial. We should also configure the `router_close_timeout` to be slightly lower than the pod's termination grace period in our deployment. A separate PR will make that setting configurable.
JAORMX
force-pushed
the
parallel-watermill
branch
from
acfd59f
to
374caf8
Compare
jhrozek
approved these changes
Oct 27, 2023
| @@ -107,3 +107,8 @@ github: | |||
| # [*] for all events. It can also be a list such as [push,branch_protection_rule]. | |||
| # Please check complete list on https://docs.github.com/es/webhooks-and-events/webhooks/webhook-events-and-payloads | |||
| events: ["*"] | |||
|
|
|||
| events: | |||
There was a problem hiding this comment.
Do we need to have these in the config.yaml? Or are they more of an illustrative example?
There was a problem hiding this comment.
(All the values are the defaults)
There was a problem hiding this comment.
We don't need to. But explicit is better than implicit
This was referenced Nov 2, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds configuration options for our watermill implementation. For now we only
have a
go-channeldriver, which is what we've been using until now. This isalso the default.
The first configuration settings this adds for the aforementioned driver as the following:
router_close_timeout: per the watermill docs this determines how long router should work for handlers when closing.buffer_size: determines the buffer size for the golang channel the driver is using.By allowing for setting the buffer size, we allow for paralellization of
request handling of events. We were not setting this before and thus
our event handling was serial.
We should also configure the
router_close_timeoutto be slightly lower thanthe pod's termination grace period in our deployment. A separate PR will make
that setting configurable.