Fix log file path based on ORCHARD_APP_DATA environment variable #15364
Conversation
src/OrchardCore/OrchardCore.Logging.NLog/HostBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.Logging.NLog/WebHostBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
| @@ -29,6 +29,7 @@ | |||
| { | |||
| "Name": "File", | |||
| "Args": { | |||
| // Replace App_Data with %ORCHARD_APP_DATA% to change log file location to use environment variable. | |||
| "path": "App_Data/logs/orchard-log.txt", | |||
There was a problem hiding this comment.
Should we also change App_Data with %ORCHARD_APP_DATA%? Then in the comment, we can explain what %ORCHARD_APP_DATA% does
There was a problem hiding this comment.
I did change - but then I reverted it - as in case of default when ORCHARD_APP_DATA is not defined, it would change the current behavior, and would create logs in ContentRoot\logs instead of ContentRoot\App_Data\logs
There was a problem hiding this comment.
I think it is a very low risk breaking change that we can adapt. We should consider adding it and then document it in the 1.9 release as a breaking change.
There was a problem hiding this comment.
It's in template affects only new sites that's created with dotnet new - IMO we should not change template with %ORCHARD_APP_DATA%
src/OrchardCore/OrchardCore.Abstractions/Setup/SetupConstants.cs
Outdated
Show resolved
Hide resolved
|
It seems that this change would require tweaking existing config file, |
| @@ -3,7 +3,7 @@ | |||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | |||
| autoReload="true" | |||
| internalLogLevel="Warn" | |||
| internalLogFile="App_Data/logs/internal-nlog.txt"> | |||
| internalLogFile="${var:configDir}/logs/internal-nlog.txt"> | |||
There was a problem hiding this comment.
Notice NLog InternalLogger is very basic, and internalLogFile="..." doesn't support parsing advanced layouts like ${var:configDir}. See also: https://github.com/NLog/NLog/wiki/Internal-Logging (Only support some basic tokens like ${basedir} or ${processdir})
| @@ -3,7 +3,7 @@ | |||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | |||
| autoReload="true" | |||
| internalLogLevel="Warn" | |||
| internalLogFile="App_Data/logs/internal-nlog.txt"> | |||
| internalLogFile="${var:configDir}/logs/internal-nlog.txt"> | |||
There was a problem hiding this comment.
Notice NLog InternalLogger is very basic, and internalLogFile="..." doesn't support parsing advanced layouts like ${var:configDir}. See also: https://github.com/NLog/NLog/wiki/Internal-Logging (Only support some basic tokens like ${basedir} or ${processdir})
|
@ns8482e can you please revisit this PR? |
|
@MikeAlhayek Thanks will look into |
…hardCMS#15364) --------- Co-authored-by: Mike Alhayek <mike@crestapps.com>
set log location to
ORCHARD_APP_DATA\logsto whenORCHARD_APP_DATAis set.Fixes #15363