-
Notifications
You must be signed in to change notification settings - Fork 34
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
Update entity samples #214
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two small suggestions, and one mistake (wrong comment) that should be easy to fix.
namespace AzureFunctionsApp; | ||
|
||
/// <summary> | ||
/// An example of performing the fibonacci sequence in Durable Functions. While this is both a (naive) recursive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment seems to be for a different sample?
|
||
public static class UserApis | ||
{ | ||
[Function("PutUser")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be helpful to start this section with a set of examples for the REST invocations, e.g.
PUT user/xyz?name=aang&age=10
GET user/xyz
DELETE user/xyz
PATCH user/xyz?age=12
POST user/xyz/greet?message=hello
/// <summary> | ||
/// Example showing the lifetime of an entity. An entity is initialized on the first operation it receives and then | ||
/// is considered deleted when <see cref="TaskEntity{TState}.State"/> is <c>null</c> at the end of an operation. | ||
/// </summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly mention:
It is also possible to design an entity that remains stateless, by overriding InitializeState to return null, and never assigning a non-null state.
Will address comments in next PR |
{ | ||
// Can also dispatch to a TaskEntity<TState> by using a static method. | ||
// However, this is a different entity ID - "counter_alt" and not "counter". Even though it uses the same | ||
// entity implementation, the function attribute has a different name, which determines the entity ID. | ||
return dispatcher.DispatchAsync<Counter>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware that we'd have two options for DispatchAsync
. If it works the way I think it does, then it makes sense. Could we add some comments briefly explaining the difference between these two options?
string? mode = request.Query["mode"]; | ||
if (int.TryParse(mode, out int m) && m == 1) | ||
if (int.TryParse(mode, out int m)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a little confusing to me since I'm not sure (as a reader of samples) whether "mode" is some concept that's important for durable entities. It looks like we're using it to specify which entity type to use. Any reason not to just accept a string value which is the entity type name directly? That would make the intent of this method more clear, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is either/or:
mode=0 or mode=entity
mode=1 or mode=state
mode=2 or mode=static
I'll expand on the comments for this.
} | ||
|
||
/// <summary> | ||
/// Optional property to override. When 'true', this will allow dispatching of operations to the TState object if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does "dispatching of operations to the TState object" mean? I don't really understand what this is describing.
|
||
public void Init() { } // no op just to init this entity. | ||
|
||
public void CustomDelete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is a CustomDelete
method needed if we have a Delete
method? Does the name have any significance? It's a little confusing to me that there are two such methods in the same entity.
|
||
public void CustomDelete() | ||
{ | ||
// Deleting an entity is done by null-ing out the state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we actually deleting the entity or just clearing its state? I wonder if we can be more explicit about what's happening.
This PR expands upon the entity samples to better demonstrate entity APIs.