Skip to content

Fix queue.max_bytes field in logstash pipeline settings spec #3925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 12, 2025

Conversation

kaiwenleee
Copy link
Contributor

Logstash PipelineSettings is using queue.max_bytes since at least 6.0 rather than queue.max_bytes.number & queue.max_bytes.units. This is also shown in the example.
Updating the spec so that elasticsearch clients can work correctly.

ref: elastic/elasticsearch#92651

Copy link

cla-checker-service bot commented Mar 10, 2025

💚 CLA has been signed

@kaiwenleee kaiwenleee force-pushed the fix/logstash-pipeline-settings branch 2 times, most recently from 84b7e8c to c6dd8eb Compare March 10, 2025 15:15
@kaiwenleee kaiwenleee force-pushed the fix/logstash-pipeline-settings branch from 9791615 to a42857e Compare March 27, 2025 13:35
@kaiwenleee kaiwenleee force-pushed the fix/logstash-pipeline-settings branch from a42857e to 599c027 Compare April 7, 2025 08:17
@kaiwenleee
Copy link
Contributor Author

kaiwenleee commented Apr 7, 2025

Hello, can any maintainer take a look at the PR, or is there something I'm missing? I did look at the contribution instruction but don't see a clear gap, except that it's no longer possible to do the validation part since clients-flight-recorder seems to be closed source now.

@l-trotta
Copy link
Contributor

hey @kaiwenleee, thanks for contributing! validation is not starting since it doesn't work for forks, I'll open a mirror PR just to start validation.

l-trotta added a commit that referenced this pull request Apr 12, 2025
@l-trotta l-trotta mentioned this pull request Apr 12, 2025
@l-trotta
Copy link
Contributor

so, turns out we're missing tests for this API ^^" but it looks correct, also this server test confirms it. LGTM, thanks again!

@l-trotta l-trotta merged commit 1403bac into elastic:main Apr 12, 2025
7 of 8 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 12, 2025
Co-authored-by: Wenkai Li <[email protected]>
Co-authored-by: Laura Trotta <[email protected]>
(cherry picked from commit 1403bac)
github-actions bot pushed a commit that referenced this pull request Apr 12, 2025
Co-authored-by: Wenkai Li <[email protected]>
Co-authored-by: Laura Trotta <[email protected]>
(cherry picked from commit 1403bac)
l-trotta pushed a commit that referenced this pull request Apr 12, 2025
… (#4273)

Co-authored-by: Wenkai Li <[email protected]>
Co-authored-by: Laura Trotta <[email protected]>
(cherry picked from commit 1403bac)

Co-authored-by: Wenkai <[email protected]>
l-trotta pushed a commit that referenced this pull request Apr 12, 2025
… (#4274)

Co-authored-by: Wenkai Li <[email protected]>
Co-authored-by: Laura Trotta <[email protected]>
(cherry picked from commit 1403bac)

Co-authored-by: Wenkai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants