Skip to content
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

Add include-coordinator to node scheduler properties docs #17366

Merged
merged 1 commit into from
May 9, 2023

Conversation

blvckcoffee
Copy link
Member

Description

  • Adds include-coordinator to the node scheduler properties reference documentation

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label May 5, 2023
@blvckcoffee blvckcoffee requested review from mosabua and colebow May 5, 2023 21:12
@blvckcoffee
Copy link
Member Author

@mosabua This property didn't seem to fit under any of the subheadings within the existing file, so I added a Configuration subheading and placed this property definition within it. Let me know if you think something else would be more appropriate or descriptive.

@github-actions github-actions bot added the docs label May 5, 2023
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Only other question is if we should link this whole properties page from the context where the property is also mention on the deploy page

docs/src/main/sphinx/admin/properties-node-scheduler.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/properties-node-scheduler.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/properties-node-scheduler.rst Outdated Show resolved Hide resolved
@blvckcoffee blvckcoffee force-pushed the jj/add-node-scheduler-props branch from fe1a8b5 to 9ef75bb Compare May 8, 2023 14:12
@blvckcoffee
Copy link
Member Author

@mosabua The Deploying Trino page has a configuration properties section that reads:

The Properties reference provides a comprehensive list of the supported properties for topics such as General properties, Resource management properties, Query management properties, Web UI properties, and others.

These node scheduler properties would be included in "others", but if you think they need to be mentioned explicitly, let me know.

Copy link
Member

@colebow colebow left a comment

Choose a reason for hiding this comment

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

LGTM, one question.

docs/src/main/sphinx/admin/properties-node-scheduler.rst Outdated Show resolved Hide resolved
@blvckcoffee blvckcoffee force-pushed the jj/add-node-scheduler-props branch from 9ef75bb to 954fbe1 Compare May 8, 2023 21:54
@mosabua
Copy link
Member

mosabua commented May 9, 2023

@mosabua The Deploying Trino page has a configuration properties section that reads:

The Properties reference provides a comprehensive list of the supported properties for topics such as General properties, Resource management properties, Query management properties, Web UI properties, and others.

These node scheduler properties would be included in "others", but if you think they need to be mentioned explicitly, let me know.

Probably ok to leave as is then

@blvckcoffee blvckcoffee force-pushed the jj/add-node-scheduler-props branch from 954fbe1 to e1c1bf4 Compare May 9, 2023 14:03
@mosabua mosabua merged commit de478d6 into trinodb:master May 9, 2023
@github-actions github-actions bot added this to the 417 milestone May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants