Skip to content

Revise entity state and status format and access#955

Merged
sebastianburckhardt merged 2 commits intofeature/core-entitiesfrom
entity-state-and-status
Sep 8, 2023
Merged

Revise entity state and status format and access#955
sebastianburckhardt merged 2 commits intofeature/core-entitiesfrom
entity-state-and-status

Conversation

@sebastianburckhardt
Copy link
Collaborator

Three changes here:

  • on the entity scheduler state: since we are maintaining the invariant (entity exists) <=> (serialized state is not null) there is no reason for defining (and serializing/deserializing) an entityExists field. Instead, just use a property.
  • in client entity helpers, provide access to helper functions so external code can deserialize strings that represent EntityStatus and SchedulerState without us having to make the serialization format public. This is needed by backends that provide entity query functionality directly.
  • also provide a shortcut (optimized) way to determine if an entity exists when looking at the entity status. Again, this is useful for backends implementing queries.

@sebastianburckhardt sebastianburckhardt added dt.core DurableTask.Core core-entities Support for entities P1 labels Sep 6, 2023
Comment on lines -27 to +28
/// <summary>
/// Whether this entity exists or not.
/// </summary>
[DataMember(Name = "exists", EmitDefaultValue = false)]
public bool EntityExists { get; set; }
[IgnoreDataMember]
public bool EntityExists => this.EntityState != null;
Copy link
Collaborator

@davidmrdavid davidmrdavid Sep 7, 2023

Choose a reason for hiding this comment

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

Is this change (changing DataMember to IgnoreDataMember) backwards compatible? Not immediately sure who's consuming this.

Copy link
Member

Choose a reason for hiding this comment

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

This change should be backwards compatible since it should always (in theory) return the same value as before. But entities haven't been shipped/announced as a public feature of DTFx, so I assume we don't need to worry about backwards compatibility yet?

Copy link
Collaborator

@davidmrdavid davidmrdavid Sep 7, 2023

Choose a reason for hiding this comment

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

entities haven't been shipped/announced as a public feature of DTFx

ah, good call out

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

so I assume we don't need to worry about backwards compatibility yet?

I actually did design the core-entities with compatibility in mind - I was planning to switch the old entity implementation in DF to the new implementation of entities in DT core, so non-isolated users can benefit from it as well. However, that is not currently our priority - for now we just care about getting Isolated working.

@sebastianburckhardt sebastianburckhardt merged commit ff23b97 into feature/core-entities Sep 8, 2023
@sebastianburckhardt sebastianburckhardt deleted the entity-state-and-status branch September 8, 2023 16:03
sebastianburckhardt added a commit that referenced this pull request Oct 17, 2023
* Bring entity logic into DurableTask.Core (first milestone) (#887)

* implementaton of entity mechanics, compatible with existing DF SDK, but without a user-facing entity SDK for DTFx

* address PR feedback.

* fix usings and namespaces

* address PR feedback

* address PR feedback (remove NameObjectManager), fix breaking change in TaskHubWorker, fix some comments

* address PR feedback (fix CustomExceptionsTest, remove public property)

* add #nullable enable to most new classes

* address PR feedback

* try to fix compiler errors

* add a configuration setting that disables separate dispatch by default

* address PR feedback

* address PR feedback

* fix semantic merge conflict.

* Revise entity state and status format and access (#955)

* update scheduler state and entity status format and helpers

* fix mess-up caused by merge conflict

* Revise entity message format and external access (#956)

* revise how event messages are represented and used

* fix merge anomaly.

* make current critical section id publicly visible (#958)

* remove orchestration tags from entity action (#952)

* Rename OperationBatchRequest and OperationBatchResponse (#953)

* rename OperationBatch to EntityBatch

* fix accidentally commited change from another PR

* Revise how entity batches are executed and handle failures (#954)

* revise task entity definition

* commit change that had been accidentally committed to a different PR.

* Apply suggestions from code review

Co-authored-by: David Justo <david.justo.1996@gmail.com>

* Apply suggestions from code review

Co-authored-by: Jacob Viau <javia@microsoft.com>

---------

Co-authored-by: David Justo <david.justo.1996@gmail.com>
Co-authored-by: Jacob Viau <javia@microsoft.com>

* revise operation result encoding and add more comments. (#965)

* revise entity backend properties and implement entity backend queries (#957)

* revise entity backend properties and implement entity backend queries.

* Minor revisions to querries and properties, and improved comments.

* fix validation of which LockNext methods are being called.

* improve comments

* fix useage of IEntityOrchestrationService.

* revise how to exclude entity results from queries.

* address PR feedback

* Update versions for ADO feed (#973)

* Add no-warn for NU5104 (#974)

* revise propagation path for entity parameters (#971)

* fix propagation path for entity parameters that need to reach the orchestration executor.

* address PR feedback.

* Revise entity queries (#981)

* rename includeDeletedEntities to includeStatelessEntities and add comment explaining the meaning

* add backlogQueueSize and lockedBy to entity metadata

* fix bugs in tracking store implementation (#979)

* add scheduled start time parameter to the start-new-orchestration operation action. (#980)

* Revise serialization of entitymessages (#972)

* revise how entity messages are serialized when sent by orchestrators.

* address PR feedback (use RawInput)

* Rename includeStateless to includeTransient in entity queries (#985)

* rename includeStateless to includeTransient

* rename variable also

* Rev to entities-preview.2 (#986)

* fix null reference exception when running on older backends (#989)

* Prepare for public preview (#994)

---------

Co-authored-by: David Justo <david.justo.1996@gmail.com>
Co-authored-by: Jacob Viau <javia@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core-entities Support for entities dt.core DurableTask.Core P1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants