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.

Sign up for GitHub

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

Jump to bottom

V11: Allow changing logging directory from configuration #13485

Merged
merged 5 commits into from
Nov 29, 2022
Merged

V11: Allow changing logging directory from configuration #13485

merged 5 commits into from
Nov 29, 2022

Conversation

bergmania
Copy link
Member

@bergmania bergmania commented Nov 28, 2022 •
edited
Loading

Related to #13356

Changes

  • Introduced new setting Umbraco.CMS.Logging.Directory
  • Obsoleted:
    • Constants.SystemDirectories.LogFiles as this is now configurable
    • A series of constructors that now points to new overloads with IOptions<LoggingSettings> parameter

Tests

  • Everything should continue to be like before this PR
  • You should now be able to write another path
    • Starting with ~ will be relative to the content root
    • Absolute paths should work
  • Test that log viewer also reads from the custom location

@bergmania bergmania marked this pull request as ready for review November 28, 2022 12:40
Zeegaan
Zeegaan requested changes Nov 28, 2022
View reviewed changes
Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

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

Found a few small changes

src/Umbraco.Infrastructure/Logging/Serilog/LoggerConfigExtensions.cs Outdated Show resolved Hide resolved
src/Umbraco.Infrastructure/Logging/Serilog/LoggerConfigExtensions.cs Outdated Show resolved Hide resolved
src/Umbraco.Core/Constants-SystemDirectories.cs Outdated Show resolved Hide resolved
@Zeegaan
Copy link
Member

Zeegaan commented Nov 28, 2022 •
edited
Loading

Setting the Umbraco/CMS/Logging/Directory and starting it with ~ results in some weird behavior for me, we now create the ~ folder, with both the Test & umbraco folder, where one contains the old logs, and the other contains the new
image

@bergmania
Copy link
Member Author

Setting the Umbraco/CMS/Logging/Directory and starting it with ~ results in some weird behavior for me, we now create the ~ folder, with both the Test & umbraco folder, where one contains the old logs, and the other contains the new image

Who would have expected a return statement to be important 🙈

@bergmania bergmania requested a review from Zeegaan November 28, 2022 17:35
@Zeegaan
Copy link
Member

Zeegaan commented Nov 29, 2022

Looks good, tests good 🎉

@Zeegaan Zeegaan merged commit bfe9579 into v11/dev Nov 29, 2022
@Zeegaan Zeegaan deleted the v11/feature/allow_changing_logging_directory_form_config branch November 29, 2022 06:28
nikolajlauridsen added a commit that referenced this pull request Dec 1, 2022
@kjac @iOvergaard
@cornehoskam @ronaldbarendse @nielslyngsoe @bergmania @Zeegaan @nikolajlauridsen @andr317c
Forward merge v11/dev to v12/dev (#13509)
0b95d5e 
* update marketplace url to its final, live url

* Added various Block Grid translations (EN & NL) (#13488)

* Added various translations

* Added another translation

* Change request

* Revert breaking changes adding IContextCache to deploy connectors (#13489)

* Remove breaking changes in Deploy connector interfaces

* Add extension methods for calling the new interface methods in a backwards-compatible way

* Create and pack empty file to add TFM dependency (#13475)

* Change login image to new SVG. (#13493)

* Only add global usings when ImplicitUsings is enabled (#13491)

* bump to rc6

* V11: Allow changing logging directory from configuration (#13485)

* Allow changing logging directory from configuration

* Clean up

* Missing return statement

* Apply suggestions from code review

Co-authored-by: Nikolaj Geisle <[email protected]>

* Update src/Umbraco.Core/Constants-SystemDirectories.cs

Co-authored-by: Nikolaj Geisle <[email protected]>

Co-authored-by: Nikolaj Geisle <[email protected]>

* Move login.svg file

* Make sure contexbar fits within the available space (#13467)

* Make sure contexbar fits within the available space

* Ensuring context bar moves above, no matter size of it.

* Fixing apperance of connection-fixers

* Info panes not loading prerequisites (#13486)

* load relations immediately on init and remove call to non-existing function `appTabChange` on destroy

* load relations immediately on init and remove now not-used eventsService

* do not set layout gaps (#13496)

* Nuget updates (#13478)

* Updated nuget packages

* Remove npoco update

Co-authored-by: nikolajlauridsen <[email protected]>

* Nuget updates (#13478)

* Updated nuget packages

* Remove npoco update

Co-authored-by: nikolajlauridsen <[email protected]>

* Bump version to final

* V11/cherry pick flaky test (#13506)

* V10/feature/fixing flaky acceptance test (#13473)

* Bumped version

* Handle external authentication providers (#13487)

Co-authored-by: Jacob Overgaard <[email protected]>
Co-authored-by: Corné Hoskam <[email protected]>
Co-authored-by: Ronald Barendse <[email protected]>
Co-authored-by: Niels Lyngsø <[email protected]>
Co-authored-by: Bjarke Berg <[email protected]>
Co-authored-by: Nikolaj Geisle <[email protected]>
Co-authored-by: nikolajlauridsen <[email protected]>
Co-authored-by: Andreas Zerbst <[email protected]>
@ronaldbarendse ronaldbarendse mentioned this pull request Dec 13, 2022
Merged
1 task
@sofietoft sofietoft mentioned this pull request Jan 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zeegaan Zeegaan Awaiting requested review from Zeegaan

Assignees
No one assigned
Labels
category/dx Developer experience release/11.1.0 type/feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bergmania @Zeegaan @nul800sebastiaan