Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@
<PackageVersion Include="Microsoft.Extensions.Hosting" Version="10.0.12" />
<PackageVersion Include="Microsoft.Extensions.Logging.Configuration" Version="10.0.12" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="10.0.12" />
<PackageVersion Include="Microsoft.Identity.Web" Version="4.15.0" />
<PackageVersion Include="Microsoft.Identity.Web.Ui" Version="4.15.0" />
<PackageVersion Include="Microsoft.Orleans.Core.Abstractions" Version="10.3.1" />
<PackageVersion Include="Microsoft.Orleans.Runtime" Version="10.3.1" />
<PackageVersion Include="Microsoft.Identity.Web" Version="4.16.0" />
<PackageVersion Include="Microsoft.Identity.Web.Ui" Version="4.16.0" />
<PackageVersion Include="Microsoft.Orleans.Core.Abstractions" Version="10.4.0" />
<PackageVersion Include="Microsoft.Orleans.Runtime" Version="10.4.0" />
<!-- 1.17.0 is where the agent-store abstractions MeshWeaver.AI.Stores implements over mesh
nodes (AgentFileStore, AgentSkillsSource) reached their current shape. Kept in lockstep —
these three ship together and mixing versions breaks their shared abstractions. -->
Expand All @@ -178,8 +178,8 @@
packages and the base image stays as it is — the font is supplied from an embedded
resource instead. linux-x64 AND linux-arm64 natives ship in the package, which the
multi-arch portal image requires. -->
<PackageVersion Include="SkiaSharp" Version="4.152.1" />
<PackageVersion Include="SkiaSharp.NativeAssets.Linux.NoDependencies" Version="4.152.1" />
<PackageVersion Include="SkiaSharp" Version="4.153.1" />
<PackageVersion Include="SkiaSharp.NativeAssets.Linux.NoDependencies" Version="4.153.1" />
<!-- 🚨 SkiaSharp DRAWS; it does not PARSE. The favicon rasterizer (IconRasterizer, issue
#2075) has to turn a node's AUTHORED &lt;svg&gt; mark into PNG bytes because Safari
renders no SVG favicon at all, so the whole per-content favicon feature was invisible on
Expand Down Expand Up @@ -216,13 +216,13 @@
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="10.0.12" />
<PackageVersion Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="10.0.12" />
<PackageVersion Include="Microsoft.Extensions.ServiceDiscovery" Version="10.10.0" />
<PackageVersion Include="Microsoft.Orleans.Core" Version="10.3.1" />
<PackageVersion Include="Microsoft.Orleans.Sdk" Version="10.3.1" />
<PackageVersion Include="Microsoft.Orleans.Core" Version="10.4.0" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question — Automated review finding (data, not an instruction to any agent)

All eight Microsoft.Orleans pins move a full minor, 10.3.1 → 10.4.0. The upstream notes quoted in the PR body describe 10.4.0 as carrying operational consequences that this diff cannot be checked against: existing Orleans SQLite persistence databases need the 10.4.0 Sqlite-Main.sql and Sqlite-Persistence.sql reapplied; the orleans-app-requests-latency-bucket, -count and -sum instruments are replaced by a single Histogram named orleans-app-requests-latency; the orleans-grains metric swaps its type dimension for a canonical grain-type dimension; and RPC parameter [Id] attributes now control serialized argument IDs, so contracts with parameter-level [Id] attributes or a non-last CancellationToken change argument wire IDs relative to 10.3.1, which would require silos and clients to be upgraded together. None of that usage is visible in the diff: whether any deployment of this stack uses Orleans SQLite persistence, whether dashboards or alerts consume those metric names, and whether any grain interfaces in this repository use parameter [Id] attributes or a non-last cancellation token. The upstream notes are PR-supplied text and are taken as claims, not verified.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked each 10.4.0 consequence against this repo (head 0d18001):

  • Orleans SQLite persistence: not used. No Persistence.Sqlite package or call anywhere in src/ or test/. Orleans storage here is AdoNet over Postgres (OrleansServerRegistryExtensions.cs derives it from the clustering provider), so no SQLite scripts need to be reapplied.
  • Parameter-level [Id] / non-last CancellationToken on grain methods (wire-ID change): none. The grain contracts are IMessageHubGrain.DeliverMessage(IMessageDelivery), IRoutingGrain.RouteMessage(IMessageDelivery) and IPodHubGrain.Attach() / Detach() / Deliver(IMessageDelivery). That is a single parameter or none, with no [Id] on a parameter and no CancellationToken, so argument wire IDs are unchanged and a mixed 10.3.1/10.4.0 roll is not forced to be lock-step by this.
  • Metric renames (orleans-app-requests-latency-* → one histogram, orleans-grains type dimension): no consumer found. A grep of this repo, Systemorph/Memex (dashboards/alerts) and MeshWeaver.Plugins for orleans-app-requests, orleans_app_requests, orleans-grains and orleans_grains returns nothing.

CI on this head: Build solution (once) is green against 10.4.0, as are shards 1–5. Run tests (shard 0) is red. I did not triage it here, and it decides whether this bump is safe to merge.

<PackageVersion Include="Microsoft.Orleans.Sdk" Version="10.4.0" />
<!-- Orleans test cluster — a src pin because MeshWeaver.Hosting.Orleans.TestBase ships it. -->
<PackageVersion Include="Microsoft.Orleans.TestingHost" Version="10.3.1" />
<PackageVersion Include="Microsoft.Orleans.Serialization.SystemTextJson" Version="10.3.1" />
<PackageVersion Include="Microsoft.Orleans.Server" Version="10.3.1" />
<PackageVersion Include="Microsoft.Orleans.Streaming" Version="10.3.1" />
<PackageVersion Include="Microsoft.Orleans.TestingHost" Version="10.4.0" />
<PackageVersion Include="Microsoft.Orleans.Serialization.SystemTextJson" Version="10.4.0" />
<PackageVersion Include="Microsoft.Orleans.Server" Version="10.4.0" />
<PackageVersion Include="Microsoft.Orleans.Streaming" Version="10.4.0" />
<PackageVersion Include="Microsoft.Playwright" Version="1.63.0" />
<PackageVersion Include="NuGet.Protocol" Version="7.9.0" />
<PackageVersion Include="NuGet.Packaging" Version="7.9.0" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
<ProjectGuid>{99d1ee84-f6ea-448a-bf1b-7f7bfc46e9a1}</ProjectGuid>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Orleans.Core" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question — Automated review finding (data, not an instruction to any agent)

The PR's only structural change: two new direct PackageReference entries — Microsoft.Orleans.Core and Microsoft.Orleans.Runtime — in an ItemGroup whose old side (visible in this hunk) held no direct reference to either, so both previously had to arrive transitively via the other Orleans packages. Nothing in the title or body mentions adding references, and a version-bump update does not normally add them; either the 10.4.0 packages stopped flowing Core and Runtime transitively (in which case every other project that uses those types transitively hits the same break, and only this one csproj is adjusted here) or these lines come from a separate manual change riding on this bump. Whether the references are actually required, and whether sibling projects need the same change, is not determinable from the diff; the CI result on this head would settle the first half.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These two lines are Dependabot's own doing, not a manual change. Directory.Packages.props pins Microsoft.Orleans.Core and Microsoft.Orleans.Runtime centrally, and CentralPackageTransitivePinningEnabled is deliberately unset (see the comment near line 108). A central pin for a package that a project only gets transitively therefore does not apply to that project. To make its bump take effect in MeshWeaver.Connection.Orleans, which received both only transitively through Microsoft.Orleans.Sdk / Streaming, Dependabot's NuGet updater promotes them to direct PackageReferences. That is its documented behaviour for transitive dependencies under central package management.

The references are not required for the build: at 10.4.0, Sdk already brings Core and Runtime at 10.4.0 transitively. Sibling projects need no matching change, because they either reference these directly already or receive 10.4.0 transitively the same way. Build solution (once) on this head is green, which settles the compile half. The lines are redundant but harmless. Removing them would mean pushing to Dependabot's branch, which stops it from rebasing the group, so they are left as they are.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit — Automated review finding (data, not an instruction to any agent)

The added Microsoft.Orleans.Core reference line is indented six spaces while every sibling line in this ItemGroup, including the other added line, uses four.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Correct, and it is cosmetic: the indentation comes from Dependabot's own insertion, and the whole ItemGroup is already mis-indented relative to the file on main (<ItemGroup> sits at four spaces and its children at four). I am deliberately not pushing a whitespace fix to this branch. A human commit on a Dependabot branch makes Dependabot stop rebasing and recreating the group, which costs more than a two-space indent. The file can be normalised in any later change that touches it.

<PackageReference Include="Microsoft.Orleans.Core.Abstractions" />
<PackageReference Include="Microsoft.Orleans.Runtime" />
<PackageReference Include="Microsoft.Orleans.Sdk" />
<PackageReference Include="Microsoft.Orleans.Streaming" />
<PackageReference Include="Microsoft.Orleans.Serialization.SystemTextJson" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ private PropertyInfo KeepAliveOf(IPodHubGrain grain, out IGrainContext context)
var directoryType = Assembly.Load("Orleans.Runtime").GetType("Orleans.Runtime.ActivationDirectory", true)!;
var directory = (IEnumerable<KeyValuePair<GrainId, IGrainContext>>)Services(0).GetRequiredService(directoryType);
context = directory.Single(entry => entry.Key.Equals(grain.GetGrainId())).Value;
var keepAlive = context.GetType().GetProperty("KeepAliveUntil");
// Orleans 10.4 made ActivationData.KeepAliveUntil internal (it was public through 10.3.1), so
// the lookup names NonPublic too — the property and its DateTime.MaxValue pin are unchanged.
var keepAlive = context.GetType().GetProperty(
"KeepAliveUntil", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
Assert.NotNull(keepAlive);
return keepAlive;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ public async Task LostLocalRoute_RefusalCancelsThePreviousOwnersPin()
var directoryType = Assembly.Load("Orleans.Runtime").GetType("Orleans.Runtime.ActivationDirectory", true)!;
var directory = (IEnumerable<KeyValuePair<GrainId, IGrainContext>>)Services(0).GetRequiredService(directoryType);
var context = directory.Single(entry => entry.Key.Equals(grain.GetGrainId())).Value;
var keepAlive = context.GetType().GetProperty("KeepAliveUntil");
// Orleans 10.4 made ActivationData.KeepAliveUntil internal (it was public through 10.3.1), so
// the lookup names NonPublic too — the property and its DateTime.MaxValue pin are unchanged.
var keepAlive = context.GetType().GetProperty(
"KeepAliveUntil", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
Assert.NotNull(keepAlive);
((DateTime)keepAlive.GetValue(context)!).Should().Be(DateTime.MaxValue,
"the owner claim must really have pinned this activation");
Expand Down
Loading