Upgrade .NET 10 #60
Upgrade .NET 10 #60
Conversation
📝 WalkthroughWalkthroughUpgrades repo to .NET 10 (SDK and target framework), bumps many package versions, removes an SQL Schema Compare file, removes an HttpClient logging extension and its DI registration, adds logger IsEnabled guards, changes test SQL container image to Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as Program.cs
participant DI as IServiceCollection
participant HttpClient as HttpClient pipeline
participant Telemetry as TelemetryTrigger
note right of Startup: Previously registered\nAddHttpClientLogger() (now removed)
Startup->>DI: services.AddHttpClientLogger() [deleted]
DI--x HttpClient: HttpClient logging hook removed
Telemetry->>Telemetry: logger.IsEnabled(LogLevel.Information)?
alt enabled
Telemetry->>Telemetry: Log Information
else disabled
Telemetry-->>Telemetry: skip Information log
end
opt exception path
Telemetry->>Telemetry: logger.IsEnabled(LogLevel.Error)?
alt enabled
Telemetry->>Telemetry: Log Error
else disabled
Telemetry-->>Telemetry: skip Error log
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/backend/Functions/Ts.Functions/TelemetryTrigger/TelemetryTrigger.cs (2)
35-38: Logger guard could be optimized for better performance gains.The
IsEnabledguard prevents theLogInformationcall when Information logging is disabled. However, thejsonstring is already computed on line 31 before this guard, so the guard doesn't prevent that potentially expensive UTF8 decoding operation.Since
jsonis also used on line 41, you could consider a different approach: only decode and log in non-production when Information logging is enabled, and always decode for the telemetry processing. However, this would complicate the flow, so the current implementation is acceptable.
48-51: Error logging guard is uncommon but acceptable.Guarding error logs with
IsEnabledchecks is unusual since error-level logging is typically always enabled in production. The guard adds a small overhead for a check that will almost always pass. However, this may be useful in specific test scenarios or as part of a consistent logging pattern across the codebase.src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj (2)
2-3: New Microsoft.Build.Sql SDK reference looks reasonable; confirm preview choice.Using the
<Sdk Name="Microsoft.Build.Sql" Version="2.1.0-preview.1" />element is consistent with multi‑SDK style; just make sure you actually want a preview build here (and that your build agents/VS version support this specific SDK version).
36-37: Security script inclusions look good; ensure files exist in source.Including
Security\create_user.sqlandSecurity\db_execute_procedure_role.sqlis fine; just confirm these files are present in the repo and don’t rely on environment‑specific assumptions (e.g., pre‑existing roles) during deployment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/backend/Database/Ts.TelemetryDatabase.Sql/Compares/SqlSchemaCompare.Dev.scmp(0 hunks)src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj(1 hunks)src/backend/Database/Ts.TelemetryDatabase/Ts.TelemetryDatabase.csproj(0 hunks)src/backend/Directory.Build.props(1 hunks)src/backend/Directory.Packages.props(1 hunks)src/backend/Functions/Ts.Functions.Tests/CacheProvider.cs(0 hunks)src/backend/Functions/Ts.Functions.Tests/Ts.Functions.Tests.csproj(0 hunks)src/backend/Functions/Ts.Functions/Extensions/HttpClientLogger.cs(0 hunks)src/backend/Functions/Ts.Functions/Program.cs(0 hunks)src/backend/Functions/Ts.Functions/TelemetryTrigger/TelemetryTrigger.cs(2 hunks)src/backend/Functions/Ts.Functions/Ts.Functions.csproj(0 hunks)src/backend/Infrastructure/Ts.Infrastructure/parameter-types.bicep(2 hunks)src/backend/Infrastructure/Ts.Infrastructure/ts.dev.bicepparam(2 hunks)src/backend/Infrastructure/Ts.Infrastructure/ts.prod.bicepparam(2 hunks)src/backend/TelemetryStash.Backend.sln(0 hunks)src/backend/TelemetryStash.Backend.slnx(1 hunks)src/backend/global.json(1 hunks)
💤 Files with no reviewable changes (8)
- src/backend/Functions/Ts.Functions/Program.cs
- src/backend/Database/Ts.TelemetryDatabase/Ts.TelemetryDatabase.csproj
- src/backend/TelemetryStash.Backend.sln
- src/backend/Functions/Ts.Functions.Tests/CacheProvider.cs
- src/backend/Functions/Ts.Functions/Ts.Functions.csproj
- src/backend/Functions/Ts.Functions.Tests/Ts.Functions.Tests.csproj
- src/backend/Database/Ts.TelemetryDatabase.Sql/Compares/SqlSchemaCompare.Dev.scmp
- src/backend/Functions/Ts.Functions/Extensions/HttpClientLogger.cs
🧰 Additional context used
🪛 GitHub Actions: CI/CD Backend Functions - Telemetry Stash
src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj
[error] 20-20: dotnet test failed. Ts.TelemetryDatabase.Sql.sqlproj could not be loaded: Name cannot begin with the '$' character, hexadecimal value 0x24. Line 20, position 36.
🔇 Additional comments (13)
src/backend/TelemetryStash.Backend.slnx (2)
1-35: LGTM: Migration to .slnx solution format.The migration from the legacy .sln format to the newer XML-based .slnx format is a good modernization step that provides better readability and source control friendliness.
25-25: Path verification complete—no issues found.The relative path
../../.github/Ts.Pipelines.GitHub.msbuildprojcorrectly navigates fromsrc/backend/to the project file, which exists at the referenced location.src/backend/Directory.Build.props (1)
3-3: LGTM: Target framework updated to .NET 10.The TargetFramework update to net10.0 is correct and aligns with the PR objectives.
src/backend/Infrastructure/Ts.Infrastructure/ts.dev.bicepparam (1)
28-28: LGTM: Runtime version consistent with production.The linuxFxVersion updates match the production configuration. Refer to the verification request in ts.prod.bicepparam to confirm the runtime string validity.
Also applies to: 39-39
src/backend/Infrastructure/Ts.Infrastructure/parameter-types.bicep (1)
42-42: LGTM: Type constraints updated for .NET 10.The linuxFxVersion type definitions now enforce
DOTNET-ISOLATED|10.0exclusively, which prevents accidental use of older runtime versions. This is appropriate for a complete framework upgrade.Also applies to: 61-61
src/backend/Directory.Packages.props (3)
20-25: LGTM: Microsoft.Extensions packages updated for .NET 10.The Microsoft.Extensions.* packages have been correctly updated from version 9.x to 10.0.0, aligning with the .NET 10 framework upgrade. Note that Microsoft.Extensions.Caching.Hybrid has also moved from preview to a stable 10.0.0 release.
9-9: Azure.Identity 1.17.1 and Microsoft.Data.SqlClient 6.1.3 are secure.Verification against GitHub Security Advisories confirms both packages are free from known vulnerabilities at their specified versions:
- Azure.Identity 1.17.1 is patched against all known HIGH and MODERATE severity advisories (RCE, information disclosure, elevation of privilege).
- Microsoft.Data.SqlClient 6.1.3 is patched against all known HIGH and MODERATE severity advisories (security feature bypasses, information disclosure).
No action required.
12-12: No issues found — both packages are compatible with .NET 10 and release notes confirm no breaking changes.Microsoft.Azure.Functions.Worker 2.51.0 has no breaking changes reported, and Microsoft.Azure.Functions.Worker.ApplicationInsights 2.50.0 similarly has no breaking changes listed. The version increments consist of feature additions and dependency updates only. The changes are safe to deploy.
src/backend/global.json (1)
3-3: .NET SDK version 10.0.100 is confirmed as a valid, stable GA release.The SDK version 10.0.100 is the official .NET 10 GA release from November 11, 2025, which is appropriate for production use.
src/backend/Infrastructure/Ts.Infrastructure/ts.prod.bicepparam (1)
29-29: Runtime string confirmed correct.The linuxFxVersion value
DOTNET-ISOLATED|10.0is the correct runtime stack identifier for .NET 10 isolated worker on Azure Functions. The code at lines 29 and 40 is accurate and requires no changes.src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj (3)
5-6: Verify new DSP/provider and Name/Guid settings match your target environment.
<Name>and the newProjectGuidare fine, but switchingDSPtoSqlAzureV12DatabaseSchemaProviderchanges the model provider semantics; double‑check this is intentional for on‑prem vs Azure deployments and matches how the DB is actually hosted.Also applies to: 9-9
28-29: Folders for Tables and UDTs included correctly.Adding
TablesandUser Defined Typesfolders keeps the project tree aligned with the Build items; no issues here.
41-41: Telemetry stored procedure and UDT Build items are consistent.Re‑adding
Stored Procedures\Telemetry\UpsertTelemetry.sqlandUser Defined Types\TelemetriesType.sqlaligns procedures with the UDT definitions and table layout; looks correct.Also applies to: 43-43
| <!-- This is necessary to ensure that VS properly restores the packages for the sql project, which is necessary for Microsoft.Build.Sql database projects. --> | ||
| <Target Name="EnsureNuGetRestore" BeforeTargets="PrepareForBuild" Condition="!Exists('$(MSBuildProjectDirectory)\obj\project.assets.json')"> | ||
| <Message Importance="high" Text="Running NuGet restore for SQL project $(MSBuildProjectName)..." /> | ||
| <Exec Command="dotnet restore "$(MSBuildProjectFile)"" ContinueOnError="false" /> | ||
| </Target> |
There was a problem hiding this comment.
🧩 Analysis chain
Fix Exec Command quoting; this causes the $ XML parse error and breaks CI.
The inner quotes in the Command attribute terminate the XML attribute early, so $(MSBuildProjectFile) is parsed as an attribute name starting with $, matching the pipeline error (“Name cannot begin with the '$' character… Line 20”). Use XML‑escaped quotes around the MSBuild property:
- <Exec Command="dotnet restore "$(MSBuildProjectFile)"" ContinueOnError="false" />
+ <Exec Command="dotnet restore "$(MSBuildProjectFile)"" ContinueOnError="false" />This should allow the project to load and dotnet test to run.
🏁 Script executed:
#!/bin/bash
# Check if the file exists and show full context around lines 17-21
fd -t f "\.sqlproj$" | head -20Length of output: 78
🏁 Script executed:
#!/bin/bash
# Verify the exact XML syntax issue with unescaped quotes
cat << 'EOF' > /tmp/test_xml.xml
<?xml version="1.0"?>
<Project>
<!-- This should fail XML parsing -->
<Exec Command="dotnet restore "$(MSBuildProjectFile)"" />
</Project>
EOF
# Try to parse it
python3 << 'PYTHON'
import xml.etree.ElementTree as ET
try:
tree = ET.parse('/tmp/test_xml.xml')
print("✗ XML parsed successfully (unexpected)")
except ET.ParseError as e:
print(f"✓ XML parsing failed as expected: {e}")
PYTHONLength of output: 84
🌐 Web query:
MSBuild Exec command proper quoting syntax for properties like $(MSBuildProjectFile)
💡 Result:
Short answer: quote the property with either XML-escaped quotes or by using single quotes for the attribute so you can use normal double-quotes inside.
Examples
- Using XML attribute with escaped quotes:
- Using single-quoted attribute (easier/readable):
Notes
- If the value can contain a trailing backslash it can interact badly with a closing quote — consider appending a dot or otherwise normalizing the path.
- If your command contains % signs or other MSBuild special chars you may need MSBuild escapes (e.g. %25 for %).
Sources: StackOverflow examples and MSBuild escaping notes. [1][2]
🏁 Script executed:
#!/bin/bash
# Extract and display lines 17-21 from the actual sqlproj file
sed -n '17,21p' 'src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj'Length of output: 507
🏁 Script executed:
#!/bin/bash
# Extract and display lines 17-21 from the actual sqlproj file
sed -n '17,21p' 'src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj'Length of output: 507
Fix Exec Command quoting to resolve XML parse error blocking CI.
Line 20 has unescaped quotes in the Command attribute that terminate the XML attribute early, causing the parser to fail on $(MSBuildProjectFile) ("Name cannot begin with the '$' character"). Use XML-escaped quotes around the MSBuild property:
- <Exec Command="dotnet restore "$(MSBuildProjectFile)"" ContinueOnError="false" />
+ <Exec Command="dotnet restore "$(MSBuildProjectFile)"" ContinueOnError="false" />🧰 Tools
🪛 GitHub Actions: CI/CD Backend Functions - Telemetry Stash
[error] 20-20: dotnet test failed. Ts.TelemetryDatabase.Sql.sqlproj could not be loaded: Name cannot begin with the '$' character, hexadecimal value 0x24. Line 20, position 36.
🤖 Prompt for AI Agents
In
src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj
around lines 17 to 21, the Exec Command attribute contains unescaped double
quotes that break XML parsing; fix the Command attribute so the inner quotes
around $(MSBuildProjectFile) are XML-safe (for example replace the inner double
quotes with "$(MSBuildProjectFile)" or switch the attribute to single
quotes and keep the inner double quotes), ensuring the resulting XML is
well-formed and the dotnet restore command runs correctly.
mikmog
force-pushed
the
upgrade-framework
branch
2 times, most recently
from
f474045 to
b9f143a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/backend/Functions/Ts.Functions/TelemetryTrigger/TelemetryTrigger.cs (1)
41-42: Improve deserialization failure message for easier diagnosticsThe exception currently interpolates
body(abyte[]), which will render asSystem.Byte[]instead of the actual payload. UsingjsonoreventData.Bodywould make the error far more actionable.- var telemetry = JsonSerializer.Deserialize<TelemetryRequest>(json, JsonOptions) - ?? throw new Exception($"Telemetry deserialization failed: {body}"); + var telemetry = JsonSerializer.Deserialize<TelemetryRequest>(json, JsonOptions) + ?? throw new Exception($"Telemetry deserialization failed. Payload: {json}");src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj (1)
8-10: Confirm LangVersion pinning in the .NET 10 world
ProjectGuidandRunSqlCodeAnalysislook fine, but explicitly pinning<LangVersion>9.0</LangVersion>here may leave this project on an older C# version than the rest of the backend now that you're on .NET 10. If there's no strong reason to lock this to 9.0, consider either dropping the property (to inherit the solution default) or bumping it to a matching/later version.The current CI error about
NuGet.Build.Tasks.Pack.targetsbeing missing doesn’t appear to originate from this sqlproj and is more likely coming from an.msbuildprojthat imports the NuGet pack SDK, so the fix for that will probably live outside this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/backend/Database/Ts.TelemetryDatabase.Sql/Compares/SqlSchemaCompare.Dev.scmp(0 hunks)src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj(1 hunks)src/backend/Database/Ts.TelemetryDatabase/Ts.TelemetryDatabase.csproj(0 hunks)src/backend/Directory.Build.props(1 hunks)src/backend/Directory.Packages.props(1 hunks)src/backend/Functions/Ts.Functions.Tests/CacheProvider.cs(0 hunks)src/backend/Functions/Ts.Functions.Tests/Ts.Functions.Tests.csproj(0 hunks)src/backend/Functions/Ts.Functions/Extensions/HttpClientLogger.cs(0 hunks)src/backend/Functions/Ts.Functions/Program.cs(0 hunks)src/backend/Functions/Ts.Functions/TelemetryTrigger/TelemetryTrigger.cs(2 hunks)src/backend/Functions/Ts.Functions/Ts.Functions.csproj(0 hunks)src/backend/Infrastructure/Ts.Infrastructure/parameter-types.bicep(2 hunks)src/backend/Infrastructure/Ts.Infrastructure/ts.dev.bicepparam(2 hunks)src/backend/Infrastructure/Ts.Infrastructure/ts.prod.bicepparam(2 hunks)src/backend/TelemetryStash.Backend.sln(0 hunks)src/backend/TelemetryStash.Backend.slnx(1 hunks)src/backend/global.json(1 hunks)
💤 Files with no reviewable changes (8)
- src/backend/Functions/Ts.Functions.Tests/Ts.Functions.Tests.csproj
- src/backend/Functions/Ts.Functions/Ts.Functions.csproj
- src/backend/Functions/Ts.Functions.Tests/CacheProvider.cs
- src/backend/Functions/Ts.Functions/Extensions/HttpClientLogger.cs
- src/backend/Database/Ts.TelemetryDatabase/Ts.TelemetryDatabase.csproj
- src/backend/Database/Ts.TelemetryDatabase.Sql/Compares/SqlSchemaCompare.Dev.scmp
- src/backend/TelemetryStash.Backend.sln
- src/backend/Functions/Ts.Functions/Program.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/backend/global.json
- src/backend/Directory.Build.props
- src/backend/Infrastructure/Ts.Infrastructure/parameter-types.bicep
- src/backend/Infrastructure/Ts.Infrastructure/ts.dev.bicepparam
- src/backend/Infrastructure/Ts.Infrastructure/ts.prod.bicepparam
🧰 Additional context used
🪛 GitHub Actions: CI/CD Backend Functions - Telemetry Stash
src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj
[error] MSBuild import failed: The imported project '/usr/share/dotnet/sdk/10.0.100/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets' was not found (MSB4019). This prevents NuGet.Build.Tasks.Pack from loading during restore/build.
[error] MSBuild import failed: The imported project '/usr/share/dotnet/sdk/10.0.100/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets' was not found (MSB4019). This prevents NuGet.Build.Tasks.Pack from loading during restore/build.
🔇 Additional comments (4)
src/backend/Functions/Ts.Functions/TelemetryTrigger/TelemetryTrigger.cs (2)
33-38: Info logging guard is clear and low-overheadThe additional
logger.IsEnabled(LogLevel.Information)guard inside the non-production block is fine and keeps the Info telemetry logging cheap when disabled, without changing behavior.
46-51: Error logging guard is consistent with the Info patternWrapping the Error log in an
IsEnabled(LogLevel.Error)check is consistent with the new pattern and keeps the hot path simple; behavior remains driven by the configured log level.src/backend/Directory.Packages.props (1)
9-27: Package bumps look aligned with .NET 10; please validate runtime and test behaviorThe Azure Functions isolated worker stack here (
Microsoft.Azure.Functions.Worker2.51.0,Worker.Sdk2.0.7,Worker.ApplicationInsights2.50.0) meets the current guidance for targeting .NET 10 isolated apps, which calls for Worker ≥ 2.50.0 and Worker.Sdk ≥ 2.0.5. (learn.microsoft.com)Upgrading
Microsoft.Extensions.*to 10.0.0 plus newerMicrosoft.Data.SqlClient,Microsoft.SqlServer.DacFx, andTestcontainers.MsSqlis a sensible match for the framework upgrade, but these jumps can change defaults (e.g., diagnostics and SQL connection behavior). It’s worth running full integration tests against a real database after this lands.On the test side,
xunit.runner.visualstudio3.1.5 is designed to run xUnit.net 2.x tests, so it should be compatible with your existingxunit2.9.3 projects alongside the newerMicrosoft.NET.Test.Sdk18.0.1. (nuget.org)Also applies to: 33-33, 35-35
src/backend/TelemetryStash.Backend.slnx (1)
23-26: The review comment is based on incorrect assumptions about CI build behaviorThe actual CI workflows do not build the slnx file. The CI only builds specific project directories directly (e.g.,
src/backend/Functions/Ts.Functions). Since the slnx file is not invoked in any active CI workflow, the presence or absence of<Build Project="false" />onTs.Pipelines.GitHub.msbuildprojhas no impact on CI success or failure.The slnx file appears to be a local development convenience, not part of the CI/CD pipeline. The suggested fix to exclude the Pipelines project from the solution descriptor would not resolve any CI issues because the CI never builds this solution file.
Likely an incorrect or invalid review comment.
mikmog
force-pushed
the
upgrade-framework
branch
from
b9f143a to
ccfac7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/backend/Database/Ts.Database.Tests/SharedTestDbFixture.cs (1)
30-30: Pin the container image to a specific versioned tag instead of usinglatest.Using
mcr.microsoft.com/mssql/server:latestintroduces non-determinism into your test suite. Microsoft's own guidance recommends explicit versioned tags for reproducibility. Different runs may pull different SQL Server versions, leading to flaky tests and making debugging harder.Update to use a versioned tag:
- .WithImage("mcr.microsoft.com/mssql/server:latest") + .WithImage("mcr.microsoft.com/mssql/server:2022-latest")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/backend/Database/Ts.Database.Tests/SharedTestDbFixture.cs(1 hunks)src/backend/Database/Ts.TelemetryDatabase.Sql/Compares/SqlSchemaCompare.Dev.scmp(0 hunks)src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj(1 hunks)src/backend/Database/Ts.TelemetryDatabase/Ts.TelemetryDatabase.csproj(0 hunks)src/backend/Directory.Build.props(1 hunks)src/backend/Directory.Packages.props(1 hunks)src/backend/Functions/Ts.Functions.Tests/CacheProvider.cs(0 hunks)src/backend/Functions/Ts.Functions.Tests/Ts.Functions.Tests.csproj(0 hunks)src/backend/Functions/Ts.Functions/Extensions/HttpClientLogger.cs(0 hunks)src/backend/Functions/Ts.Functions/Program.cs(0 hunks)src/backend/Functions/Ts.Functions/TelemetryTrigger/TelemetryTrigger.cs(2 hunks)src/backend/Functions/Ts.Functions/Ts.Functions.csproj(0 hunks)src/backend/Infrastructure/Ts.Infrastructure/parameter-types.bicep(2 hunks)src/backend/Infrastructure/Ts.Infrastructure/ts.dev.bicepparam(2 hunks)src/backend/Infrastructure/Ts.Infrastructure/ts.prod.bicepparam(2 hunks)src/backend/TelemetryStash.Backend.sln(0 hunks)src/backend/TelemetryStash.Backend.slnx(1 hunks)src/backend/global.json(1 hunks)
💤 Files with no reviewable changes (8)
- src/backend/Functions/Ts.Functions/Ts.Functions.csproj
- src/backend/Functions/Ts.Functions.Tests/CacheProvider.cs
- src/backend/Functions/Ts.Functions.Tests/Ts.Functions.Tests.csproj
- src/backend/Functions/Ts.Functions/Program.cs
- src/backend/Database/Ts.TelemetryDatabase/Ts.TelemetryDatabase.csproj
- src/backend/Database/Ts.TelemetryDatabase.Sql/Compares/SqlSchemaCompare.Dev.scmp
- src/backend/Functions/Ts.Functions/Extensions/HttpClientLogger.cs
- src/backend/TelemetryStash.Backend.sln
🚧 Files skipped from review as they are similar to previous changes (5)
- src/backend/Functions/Ts.Functions/TelemetryTrigger/TelemetryTrigger.cs
- src/backend/global.json
- src/backend/Infrastructure/Ts.Infrastructure/ts.prod.bicepparam
- src/backend/Directory.Packages.props
- src/backend/Directory.Build.props
🔇 Additional comments (9)
src/backend/Infrastructure/Ts.Infrastructure/parameter-types.bicep (1)
42-44: Runtime type updated to .NET 10 isolated — looks consistentBoth
appServiceParams.linuxFxVersionandfunctionParams.linuxFxVersionare now pinned to'DOTNET-ISOLATED|10.0', which matches the .NET 10 upgrade and keeps the parameter types strict so mismatched values will fail at compile time instead of at deploy/runtime.Just make sure your target App Service/Functions stacks in the deployed regions currently support
DOTNET-ISOLATED|10.0so these deployments don’t get blocked in some environments.Also applies to: 61-63
src/backend/Infrastructure/Ts.Infrastructure/ts.dev.bicepparam (1)
19-42: Dev app runtimes aligned to .NET 10 isolatedUpdating
appParameters.apiandappParameters.functionslinuxFxVersionto'DOTNET-ISOLATED|10.0'keeps the dev environment in sync with the new parameter types and the overall .NET 10 move. This looks good and avoids drift between infra definition and app runtime.If you haven’t already, double‑check that the target dev subscription/region exposes this runtime so these deployments don’t start failing once merged.
src/backend/Database/Ts.Database.Tests/SharedTestDbFixture.cs (2)
37-86: LGTM!The database provisioning logic is sound: proper resource management, safe connection string building, and appropriate test user setup.
88-97: LGTM!The async lifecycle management is correctly implemented with proper cleanup and appropriate use of
ConfigureAwait(false).src/backend/TelemetryStash.Backend.slnx (3)
25-25: The Pipelines project path resolves correctly and the file exists.The referenced file
.github/Ts.Pipelines.GitHub.msbuildprojexists at the repository root and the relative path../../.github/Ts.Pipelines.GitHub.msbuildprojfromsrc/backend/TelemetryStash.Backend.slnxresolves correctly. No action needed.
19-21: No action needed —Build Project="false"is correct for this project.The Infrastructure project uses the
Microsoft.Build.NoTargetsSDK and contains only file inclusions (no build targets or tasks). TheBuild="false"setting is intentional and appropriate; this project serves as a file container for infrastructure resources and should not be built during solution builds.
1-35: All referenced project paths, README files, and solution item files exist and resolve correctly. The solution file structure is valid with no missing references.src/backend/Database/Ts.TelemetryDatabase.Sql/Ts.TelemetryDatabase.Sql.sqlproj (2)
2-2: No action needed — SDK and DSP versions are compatible with .NET 10.The upgrade to
Microsoft.Build.Sql/1.0.0andSql170DatabaseSchemaProvider(SQL Server 2022) is compatible with .NET 10. Microsoft.Build.Sql/1.0.0 requires .NET 8 or higher, and the DacFx 170.x release explicitly targets net10.0 compatibility. These are the appropriate modern versions for .NET 10 projects.
1-47: The XML quoting issue has been resolved—the problematic Exec command has been removed.The file contains no Exec elements and parses as valid, well-formed XML. The issue that was blocking CI in the past review has been fully addressed by removing the problematic code.
Summary by CodeRabbit
Chores
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.