Enhancement to respect user-defined log.path in launcher.py #73
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
|
@tdcmeehan @steveburnett @imjalpreet please review. |
|
Please add a release note entry if necessary, following the release notes guidelines. |
|
@steveburnett done |
|
@tdcmeehan please review. |
|
@tdcmeehan please review @imjalpreet @ajaygeorge |
|
@tdcmeehan @steveburnett @imjalpreet @ajaygeorge please review. |
|
Please review the Order of Changes in the Presto Release Notes Guidelines, and revise your current release note entry appropriately. |
|
@tdcmeehan please review. |
|
This is blocking prestodb/presto#22271. |
|
Please help to get this merged
…On Thu, 12 Sep 2024 at 8:06 PM, Steve Burnett ***@***.***> wrote:
This is blocking prestodb/presto#22271
<prestodb/presto#22271>.
—
Reply to this email directly, view it on GitHub
<#73 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AVL2AZXTQ5QHI4H2YFOAJFLZWGRHZAVCNFSM6AAAAABFCZAE6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBWGQ4DGMBQG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses an enhancement in launcher.py to handle the log.path setting from node.properties. Currently, regardless of the value set for log.path in node.properties, the launcher.py script always uses the default value. This behavior is not correct as it does not respect the user's configuration.
Context:
This issue arises when launcher.py is run and the log.path is explicitly provided by the user in node.properties. The script does not respect this setting and continues to use the default value.
Motivation:
The motivation for this enhancement is to respect the user's explicit configuration of log.path in node.properties. The user should have the ability to control the configuration from their end. This change will make the script more adaptable to different user configurations and improve the user experience.
Understanding:
To implement this enhancement, the o.server_dir variable, which is used to construct the server log path, is set to the value of log.path from node.properties when it's explicitly provided by the user. This allows the user's setting to override the default value.
If log.path is not provided in node.properties, o.server_dir is set to a default path. This ensures that o.server_dir is always a valid path, even when log.path is not provided.
Details:
The issue arises when the log.path is explicitly set by the user in node.properties and the service is restarted. Despite the user's explicit configuration, the system prints pid -Dlog.path= instead of the user-defined value.
Test plan :
== RELEASE NOTES ==
General Changes
*Improve launcher.py to respect user-defined log.path.