-
Notifications
You must be signed in to change notification settings - Fork 17
Add package and API skeleton for internal queue #40
Conversation
Looks good as a starting point. Tests are coming later I assume? |
Yeah, I just wanted to lock in the structure that isn't dependent on the outstanding libbeat api change, I'm expecting some unit tests when that gets merged and probably additional end-to-endish ones following that to test the whole process. |
package queue | ||
|
||
import ( | ||
beatsqueue "github.com/elastic/beats/v7/libbeat/publisher/queue" |
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.
Why are you using this? Is this a temporary import and going to be removed in a later relase?
I was under the impression that we are trying to avoid importing from beats in our new repositories. If we want to reuse something from beats we should either move it to elastic-agent-libs
or copy it to the new repo where it is used.
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 shipper is initially going to be built out of the beats outputs, queues, and the code that glues them together. That is a large chunk of code to move that at this point would just slow down development. I would like us to prioritize getting a working shipper prototype over the code reorganization right now.
I am in favour of moving all that code into elastic-agent-libs
afterwards or perhaps in the background. That will make it clear that the code is used outside of just beats now.
I will create an issue to migrate the code we end up using out of beats, which I assume will also be easier once we know what pieces we need to reuse.
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, there is still more pending work to clean up and modularize the queue and I expect that as that progresses it will probably migrate to elastic-agent-libs
, it just wasn't part of the first draft.
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.
This is the first pass of integrating the libbeat queue into the shipper server. It creates the package namespace and structures and passes through the initialization / shutdown process, but doesn't yet use a real queue internally (that is waiting on the libbeat-side api changes to be merged in elastic/beats#31699). But this creates a skeleton and moves things to the right places so we can keep building around it without version skew from the draft PRs.