-
Notifications
You must be signed in to change notification settings - Fork 17
Add Disk Queue configuration #138
Add Disk Queue configuration #138
Conversation
This pull request is now in conflicts. Could you fix it? 🙏
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
- runs server in stand alone mode - checks that server starts - checks that it fails to start with bad config - checks that a message can be published and is written to output
276e948
to
aac1a29
Compare
3663b99
to
b402062
Compare
- config options taken from beats - add `use_compression` options - add `use_encryption` option - add `encryption_password` option for stand alone and testing, elastic-agent use case will query for encryption key (to do). Closes elastic#119
b402062
to
31394c3
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.
Generally looks good, a few questions about tests and how we should get the encryption key.
Please review elastic/elastic-agent#1527 and make sure the configuration there is in sync with this one. Likely this we have another variant of the shared configuration problem between the agent and the shipper, where we have no way to guarantee the two configs stay in sync yet.
integration/integration_test.go
Outdated
"string": unique, | ||
"number": 42, | ||
events, err := createEvents([]string{unique}) | ||
if err != nil { |
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.
Might be best to use the "github.com/stretchr/testify/require"
library we use everywhere else; will clean up the tests at least.
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.
reworked with require
, also added an environment Fatalf, so when the test fails you see the stdout & stderr so you have some idea what happened.
integration/integration_test.go
Outdated
Fields: sampleValues, | ||
} | ||
events := []*messages.Event{e} | ||
client, err := env.NewClient("localhost:50052") |
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.
A little worried that the hard-coded ports are going to collide with something, but it looks like the rest of the tests already do that, so unless you're particularly interested in modifying the framework to assign random free ports, that's probably a task for another day.
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.
yeah it is a risk. Going to wait and see if it is a problem. If it is, my guess is we should move to Docker for starting the Shipper.
444c219
to
bacb596
Compare
What does this PR do?
In configuration file we will have:
These are the same as beats except for
use_compression
, andencryption_password
. Which are new tosupport encryption & compression.
Why is it important?
Need to be able to configure the disk queue to use it in the shipper.
Checklist
CHANGELOG.md
orCHANGELOG-developer.md
.How to test this PR locally
mage integrationTest
Related issues