Skip to content

Move DurableTaskRegistry to Abstractions - #76

Merged
Jacob Viau (jviau) merged 3 commits into
microsoft:mainfrom
jviau:move-registry
Nov 10, 2022
Merged

Move DurableTaskRegistry to Abstractions#76
Jacob Viau (jviau) merged 3 commits into
microsoft:mainfrom
jviau:move-registry

Conversation

@jviau

@jviauJacob Viau (jviau) commented Nov 10, 2022

Copy link
Copy Markdown
Member

Overview

This PR moves DurableTaskRegistry to Abstractions assembly. The reason for this move is for the code-gen scenario. It is important to have access to the registry from just the abstractions package, so registration methods can be generated.

Summary of Changes

  • DurableTaskRegistry moved to Abstractions assembly.
  • DurableTaskRegistry.Build moved to an extension method BuildFactory in worker assembly.
  • Internal dictionaries on the registry were changed to IDictionary.
  • Systems.Collection.Immutable removed from Abstractions.
    • Systems.Collection.Immutable was added to Worker.Grpc, but that is already used by another nuget package. This addition is to resolve a version mismatch.
  • System.Collections.Concurrent removed - it was unused.
  • Abstractions internals were made visible to Worker and Client projects.
    • Not the gRPC implementations, so we do not accidentally depend on functionality other worker implementations would not have access to.
    • Had to adjust some shared sections to avoid type duplication.

cgillum
Chris Gillum (cgillum) approved these changes Nov 10, 2022
View reviewed changes

@cgillumChris Gillum (cgillum) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall, LGTM!

Comment threadsrc/Abstractions/DurableTaskRegistry.cs Outdated
Comment threadsrc/Abstractions/DurableTaskRegistry.cs
@jviau
Jacob Viau (jviau) merged commit 4d12d5a into microsoft:mainNov 10, 2022
@jviau
Jacob Viau (jviau) deleted the move-registry branch November 10, 2022 21:36
@YunchuWangwangbill (YunchuWang) mentioned this pull request Jul 8, 2026
Open
wangbill (YunchuWang) added a commit that referenced this pull request Aug 18, 2026
@YunchuWang
Correct stale purge-disposition docs left by the F1 quarantine change
ce3b547
The F1 change (a814ffc) made an unowned/unmarked blob report Quarantined
instead of Deleted, but several comments and XML docs still described the
old Deleted-covers-unowned behavior. Doc/comment only - no product logic
or test-logic change.
- src/Grpc/orchestrator_service.proto: DELETED now covers two cases (blob
deleted, blob already absent), not three; the "left in place because
unowned" case is QUARANTINED now. Language-neutral wire semantics only;
the same hunk mirrors into microsoft/durabletask-protobuf #76.
- src/Client/Core/LargePayloadPurgeDisposition.cs: same two-case fix on the
C# mirror of that enum ("all three cases" -> "both cases").
- src/Extensions/AzureBlobPayloads/PayloadStore/PayloadDeleteOutcome.cs: the
NotStoreOwned summary no longer claims the reference "is still resolved";
it states the store-layer fact plus a neutral note that the outcome is not
proof of deletion and the caller decides how to dispose of the reference.
No disposition named - that mapping is the activity's decision.
- src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs: same
correction at the HasOwnershipMarker return site.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6373d1ba-104b-492a-9737-456a26c8ea34
wangbill (YunchuWang) added a commit that referenced this pull request Aug 18, 2026
@YunchuWang
Revert F1: unowned blobs report Deleted again, not Quarantined
7669cf9
The user overruled F1. The backend treats a non-empty quarantine set as a
poison/investigate signal: it emits an OTel-flagged
LargePayloadPurgeQuarantineOutstanding on every GetLargePayloadTombstones
poll (once per IdleDelay = 1 minute, forever), quarantine never auto-expires,
and there is no operator tool to clear it. Because the managed_by=dts
ownership marker is new in this PR, every pre-existing v2 blob would hit
NotStoreOwned on upgrade and permanently wedge that alarm. So an
unowned/unmarked blob reports Deleted (terminal success) again, as before F1.
Reverts F1 only. F2 (the disable path keeps reconciling) and F3 (the two
client-facing purge APIs stay removed) are unchanged.
- DeleteExternalBlobActivity.cs, Logs.cs, DeleteExternalBlobActivityTests.cs:
restored byte-identical to their pre-F1 (0034bb0) state. NotStoreOwned
logs BlobPurgeBlobNotStoreOwned (EventId 821 restored in its 820-822 slot)
and falls through to Deleted; the test asserts Deleted again.
- orchestrator_service.proto, LargePayloadPurgeDisposition.cs,
PayloadDeleteOutcome.cs, BlobPayloadStore.cs: the ce3b547 doc sweep only
existed to describe the quarantine semantics, so it is reverted too -
restored byte-identical to 9be9931, back to the three-case DELETED wording
that matches Deleted semantics and the untouched protobuf #76 copy.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6373d1ba-104b-492a-9737-456a26c8ea34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgillumChris Gillum (cgillum)cgillum approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jviau@cgillum