Skip to content

Fix redis-hbo-provider docs and add coordinator hbo configuration#21477

Merged
tdcmeehan merged 1 commit intoprestodb:masterfrom
jaystarshot:hbo-docs-fix
Dec 5, 2023
Merged

Fix redis-hbo-provider docs and add coordinator hbo configuration#21477
tdcmeehan merged 1 commit intoprestodb:masterfrom
jaystarshot:hbo-docs-fix

Conversation

@jaystarshot
Copy link
Member

@jaystarshot jaystarshot commented Dec 4, 2023

Fixes #21474

[DOC change only]

Correct the documentation where the wrong configuration file name was mentioned. Include the properties for HBO configuration on the coordinator.

Motivation and Context

Impact

Test Plan

Contributor checklist

== RELEASE NOTES ==

General Changes
* Fixed redis-provider-plugin docs which previously contained an incorrect file name for property loading, and included the HBO configurations for the coordinator.

@github-actions github-actions bot added the docs label Dec 4, 2023
@jaystarshot jaystarshot force-pushed the hbo-docs-fix branch 2 times, most recently from 5d02cbe to 2558aeb Compare December 4, 2023 19:20
@jaystarshot jaystarshot marked this pull request as ready for review December 4, 2023 19:21
@jaystarshot jaystarshot requested a review from a team as a code owner December 4, 2023 19:21
@github-actions
Copy link

github-actions bot commented Dec 4, 2023

Codenotify: Notifying subscribers in CODENOTIFY files for diff 76ae3ed...3589289.

Notify File(s)
@steveburnett presto-docs/src/main/sphinx/plugin/redis-hbo-provider.rst

@jaystarshot jaystarshot force-pushed the hbo-docs-fix branch 2 times, most recently from 5666bc3 to 39cb895 Compare December 4, 2023 19:28
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! (docs)

Local build of docs, reviewed it with #21474 in mind and these changes address the concerns raised there.

I would suggest that you consider, in the PR, deleting everything in the Release Notes section below the line "Fixed redis-provider-plugin docs...". But that's separate from the docs content that I've reviewed.

Thanks!

@steveburnett
Copy link
Contributor

Thanks for updating the release notes entry!

@jaystarshot
Copy link
Member Author

@steveburnett I added documentation for 2 new configs too in the optimizer part. Thanks!

@jaystarshot jaystarshot force-pushed the hbo-docs-fix branch 2 times, most recently from e9ac309 to 7f728a4 Compare December 5, 2023 02:04
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! (docs)

Reviewed new updated docs in a local build, everything looks good. Thanks!

@steveburnett
Copy link
Contributor

@steveburnett I added documentation for 2 new configs too in the optimizer part. Thanks!

Re-reviewed, and approved. Thanks!

@tdcmeehan tdcmeehan merged commit 0dfc35e into prestodb:master Dec 5, 2023
@wanglinsong wanglinsong mentioned this pull request Feb 12, 2024
64 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Documentation - Could you kindly give a link of the HBO plugin jars?

3 participants