-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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 \log.path\
Configuration Property to Presto
#22271
base: master
Are you sure you want to change the base?
Conversation
@tdcmeehan please review @steveburnett |
Codenotify: Notifying subscribers in CODENOTIFY files for diff 2216cc6...db0b26f.
|
Please update the release note entry to
|
@tdcmeehan, the doc here looks good in a local build. Asking you to review to check the technical accuracy of the words, given your involvement with prestodb/airlift#71. |
It's accurate, but it shoulnd't be merged until a new airlift release with prestodb/airlift#71 is cut. |
https://github.com/prestodb/airlift/pull/73/files -we need this also for proper. @steveburnett @tdcmeehan |
Do you mean that this PR should not be merged until a new airlift release that includes both prestodb/airlift#71 and prestodb/airlift#73 is cut? |
Yes
…On Fri, 22 Mar 2024 at 7:53 PM, Steve Burnett ***@***.***> wrote:
https://github.com/prestodb/airlift/pull/73/files -we need this also for
proper. @steveburnett <https://github.com/steveburnett> @tdcmeehan
<https://github.com/tdcmeehan>
Do you mean that this PR should not be merged until a new airlift release
that includes both prestodb/airlift#71
<prestodb/airlift#71> and prestodb/airlift#73
<prestodb/airlift#73> is cut?
—
Reply to this email directly, view it on GitHub
<#22271 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AVL2AZQGVTWBZF2ZQXVIP4TYZQ5F7AVCNFSM6AAAAABFBKOCD2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJVGIYTMNJSGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Description
This PR adds a new configuration property
log.path
to Presto. This property specifies the path to the log file used by Presto. The path is relative to the data directory. This addition comes after the changes introduced in Pull Request 71.Motivation and Context
The motivation behind this change is to give users the ability to specify their own path for the server log file. This can be useful in scenarios where the default log file location needs to be changed due to system constraints or personal preference.
Impact
no impact.
Test Plan
Add the
log.path
property to the Presto configuration.2. Start Presto and check the specified log file path to ensure that logging is happening at the correct location.
3. Change the
log.path
property value and restart Presto to ensure the new value is taken into effect.Other Details:
This PR needs to be merged along with a related PR in the airlift repository. The airlift PR adjusts the launcher.py script to honor the log.path property. At present, even if we set this property, the default values from launcher.py override it, as shown in the attached screenshots. This is incorrect behavior - if a user sets the log.path property, that value should be used instead of the default.
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
If release note is NOT required, use: