-
Notifications
You must be signed in to change notification settings - Fork 26
Add caching for Kroki diagrams #1601
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
Open
theletterf
wants to merge
15
commits into
main
Choose a base branch
from
theletterf-add-kroki-caching
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
e71e3ba
Add caching
theletterf 04ac6c1
Fix various issues
theletterf 1896b71
Fix lint errors
theletterf 1383eb0
Add callout
theletterf 8d0e543
Fix test
theletterf b205ac2
Fix for slashes (hopefully)
theletterf 5eae0cd
Tech review changes
theletterf d8e6429
Edits
theletterf e56d078
Merge branch 'main' into theletterf-add-kroki-caching
theletterf a92e38e
Merge branch 'main' into theletterf-add-kroki-caching
theletterf 4e3a416
Move registry to documentation set
Mpdreamz b4c88de
Fail the build on CI if new cachable SVG files are discovered
Mpdreamz d84ace3
register output file in output folder, we have to write it to both so…
Mpdreamz 2d7aae9
Merge branch 'main' into theletterf-add-kroki-caching
theletterf 4051b97
Merge branch 'main' into theletterf-add-kroki-caching
theletterf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
21 changes: 21 additions & 0 deletions
21
src/Elastic.Markdown/Myst/Directives/Diagram/DiagramHttpClient.cs
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// Licensed to Elasticsearch B.V under one or more agreements. | ||
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. | ||
// See the LICENSE file in the project root for more information | ||
|
||
namespace Elastic.Markdown.Myst.Directives.Diagram; | ||
|
||
/// <summary> | ||
/// Shared HttpClient for diagram downloads to avoid resource exhaustion | ||
/// </summary> | ||
public static class DiagramHttpClient | ||
theletterf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
private static readonly Lazy<HttpClient> LazyHttpClient = new(() => new HttpClient | ||
{ | ||
Timeout = TimeSpan.FromSeconds(30) | ||
}); | ||
|
||
/// <summary> | ||
/// Shared HttpClient instance for diagram downloads | ||
/// </summary> | ||
public static HttpClient Instance => LazyHttpClient.Value; | ||
} |
146 changes: 146 additions & 0 deletions
146
src/Elastic.Markdown/Myst/Directives/Diagram/DiagramRegistry.cs
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
// Licensed to Elasticsearch B.V under one or more agreements. | ||
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. | ||
// See the LICENSE file in the project root for more information | ||
|
||
using System.IO.Abstractions; | ||
|
||
namespace Elastic.Markdown.Myst.Directives.Diagram; | ||
|
||
/// <summary> | ||
/// Registry to track active diagrams and manage cleanup of outdated cached files | ||
/// </summary> | ||
public static class DiagramRegistry | ||
theletterf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
private static readonly HashSet<string> ActiveDiagrams = []; | ||
theletterf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private static readonly Lock Lock = new(); | ||
|
||
/// <summary> | ||
/// Register a diagram as active during the current build | ||
/// </summary> | ||
/// <param name="localSvgPath">The local SVG path relative to output directory</param> | ||
public static void RegisterDiagram(string localSvgPath) | ||
{ | ||
if (string.IsNullOrEmpty(localSvgPath)) | ||
return; | ||
|
||
lock (Lock) | ||
{ | ||
_ = ActiveDiagrams.Add(localSvgPath); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Get all currently registered active diagrams | ||
/// </summary> | ||
/// <returns>Collection of active diagram paths</returns> | ||
public static IReadOnlyCollection<string> GetActiveDiagrams() | ||
{ | ||
lock (Lock) | ||
{ | ||
return ActiveDiagrams.ToArray(); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Clear all registered diagrams (typically called at start of build) | ||
/// </summary> | ||
public static void Clear() | ||
theletterf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
lock (Lock) | ||
theletterf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
ActiveDiagrams.Clear(); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Clean up unused diagram files from the output directory | ||
/// </summary> | ||
/// <param name="outputDirectory">The output directory path</param> | ||
/// <returns>Number of files cleaned up</returns> | ||
public static int CleanupUnusedDiagrams(string outputDirectory) => | ||
theletterf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
CleanupUnusedDiagrams(outputDirectory, new FileSystem()); | ||
|
||
/// <summary> | ||
/// Clean up unused diagram files from the output directory | ||
/// </summary> | ||
/// <param name="outputDirectory">The output directory path</param> | ||
/// <param name="fileSystem">File system abstraction for testing</param> | ||
/// <returns>Number of files cleaned up</returns> | ||
public static int CleanupUnusedDiagrams(string outputDirectory, IFileSystem fileSystem) | ||
{ | ||
if (string.IsNullOrEmpty(outputDirectory)) | ||
return 0; | ||
|
||
var graphsDir = fileSystem.Path.Combine(outputDirectory, "images", "generated-graphs"); | ||
if (!fileSystem.Directory.Exists(graphsDir)) | ||
return 0; | ||
|
||
var cleanedCount = 0; | ||
var activePaths = GetActiveDiagrams(); | ||
|
||
try | ||
{ | ||
var existingFiles = fileSystem.Directory.GetFiles(graphsDir, "*.svg", SearchOption.AllDirectories); | ||
|
||
foreach (var file in existingFiles) | ||
{ | ||
var relativePath = fileSystem.Path.GetRelativePath(outputDirectory, file); | ||
|
||
// Convert to forward slashes for consistent comparison | ||
var normalizedPath = relativePath.Replace(fileSystem.Path.DirectorySeparatorChar, '/'); | ||
|
||
if (!activePaths.Any(active => active.Replace(fileSystem.Path.DirectorySeparatorChar, '/') == normalizedPath)) | ||
{ | ||
try | ||
{ | ||
fileSystem.File.Delete(file); | ||
cleanedCount++; | ||
} | ||
catch | ||
{ | ||
// Silent failure - cleanup is opportunistic | ||
} | ||
} | ||
} | ||
|
||
// Clean up empty directories | ||
CleanupEmptyDirectories(graphsDir, fileSystem); | ||
} | ||
catch | ||
{ | ||
// Silent failure - cleanup is opportunistic | ||
} | ||
|
||
return cleanedCount; | ||
} | ||
|
||
/// <summary> | ||
/// Remove empty directories recursively | ||
/// </summary> | ||
/// <param name="directory">Directory to clean up</param> | ||
/// <param name="fileSystem">File system abstraction</param> | ||
private static void CleanupEmptyDirectories(string directory, IFileSystem fileSystem) | ||
{ | ||
try | ||
{ | ||
if (!fileSystem.Directory.Exists(directory)) | ||
theletterf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return; | ||
|
||
// Clean up subdirectories first | ||
foreach (var subDir in fileSystem.Directory.GetDirectories(directory)) | ||
{ | ||
CleanupEmptyDirectories(subDir, fileSystem); | ||
} | ||
|
||
// Remove directory if it's empty | ||
if (!fileSystem.Directory.EnumerateFileSystemEntries(directory).Any()) | ||
{ | ||
fileSystem.Directory.Delete(directory); | ||
} | ||
} | ||
catch | ||
{ | ||
// Silent failure - cleanup is opportunistic | ||
} | ||
} | ||
} |
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.