Skip to content
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

Added .NET example to jobs HowTo page #4385

Closed
wants to merge 3 commits into from
Closed

Conversation

WhitWaldo
Copy link
Contributor

Thank you for helping make the Dapr documentation better!

Please follow this checklist before submitting:

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Read the contribution guide
  • Commands include options for Linux, MacOS, and Windows within codetabs
  • New file and folder names are globally unique
  • Page references use shortcodes instead of markdown or URL links
  • Images use HTML style and have alternative text
  • Places where multiple code/command options are given have codetabs

In addition, please fill out the following to help reviewers understand this pull request:

Description

Added .NET sample to the Jobs API documentation originally merged in #4376

@WhitWaldo
Copy link
Contributor Author

Please do not merge until dapr/dotnet-sdk#1331 is accepted

@msfussell msfussell added this to the 1.15 milestone Oct 12, 2024
@msfussell msfussell added area/jobs waiting-on-code-pr The code PR needs to be merged before the docs are updated labels Oct 12, 2024

The job is using the free-form `@every` expressions to let the developer indicate that the job should be triggered `@every 1s` and sets the optional `repeats` argument to indicate the maximum number of times the job should be triggered and sent back to the app; in this scenario, we set a value of 10 indicating that the job should be triggered a maximum of ten times.

At the trigger time, the registered handler will be invoked and will execute the desired business logic for the job. In this sample, we apply a switch statement with a value known at compile-time, but in a real-world app, this would very well perform a more dynamic service lookup in order to determine how any given job name should be handled.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
At the trigger time, the registered handler will be invoked and will execute the desired business logic for the job. In this sample, we apply a switch statement with a value known at compile-time, but in a real-world app, this would very well perform a more dynamic service lookup in order to determine how any given job name should be handled.
At the trigger time, the registered handler is invoked and executes the desired business logic for the job. In this example, a switch statement is used with a value known at compile-time, but in typical production code it is preferred to have a dynamic service lookup in order to determine how any given job name should be handled.

Copy link
Member

Choose a reason for hiding this comment

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

It would be good to explain DaprJobDetails and how this is used

Copy link
Contributor Author

@WhitWaldo WhitWaldo Oct 12, 2024

Choose a reason for hiding this comment

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

Certainly. I think I may rewrite some chunks of this to also provide a better example for DaprJobSchedule. In this writing, I was just mirroring the schedule used by the Go demo, but there are some helper methods in this class that should help developers properly format these expressions without necessarily knowing the various expression syntaxes.

I also mildly dispute the last suggestion as this is code simply presented as a simple proof-of-concept. Ideally, production code mirrors development code, so I would assert it is a better fit to refer to this as real-world vs demonstration code as opposed to the suggested "production code".


At the trigger time, the registered handler will be invoked and will execute the desired business logic for the job. In this sample, we apply a switch statement with a value known at compile-time, but in a real-world app, this would very well perform a more dynamic service lookup in order to determine how any given job name should be handled.

The `MapDaprScheduledJobHandler` accepts a delegate that works much like the ASP.NET Core minimal API parameter binding logic. The first parameter will always contain the name of the job, the second will reflect the job details provided in the callback and the last parameter must be a cancellation token. Otherwise, the developer is free to provide any additional services registered via dependency injection before the cancellation token argument and they will be injected at runtime.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `MapDaprScheduledJobHandler` accepts a delegate that works much like the ASP.NET Core minimal API parameter binding logic. The first parameter will always contain the name of the job, the second will reflect the job details provided in the callback and the last parameter must be a cancellation token. Otherwise, the developer is free to provide any additional services registered via dependency injection before the cancellation token argument and they will be injected at runtime.
The `MapDaprScheduledJobHandler` accepts a delegate that works much like the ASP.NET Core minimal API parameter binding logic. The first parameter is the name of the job, the second parameter is job details provided in the callback and the last parameter is a cancellation token. You are free to provide any additional services registered via dependency injection before the cancellation token argument and they are injected at runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with this except "the second parameter is job details" doesn't flow right to me. Would you be more accepting of "the second parameter contains job details" with a description in a prior paragraph of what those job details might look like?

//Do something
};

// Set a handler for incoming jobs
Copy link
Member

Choose a reason for hiding this comment

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

Little confused on this comment. This is "configure a map of handlers that are then called" Trying to be clear where the call back from the jobs schedule happens, since that is in JobsController yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it's the problem with making condensed examples in that I wanted to have a feature-complete demo though in practice, the handler itself would likely be handled in an injected service. I was planning on updating the example to have a more feature-complete sample (and perhaps link to it from here) that shows a more real-world scenario.

Because the Go example in this documentation splits between the SDK, HTTP, and gRPC samples (with a blurb calling out SDKS again at the bottom), I was trying to follow the same format wherein the SDK favors DI-based Jobs client registration and ASP.NET Core mapping (but then have a one-file demo) and then put the HTTP version in the controllers.

…bs/howto-schedule-and-handle-triggered-jobs.md

Co-authored-by: Mark Fussell <[email protected]>
Signed-off-by: Whit Waldo <[email protected]>
@WhitWaldo
Copy link
Contributor Author

I inadvertently saved this to the wrong branch locally which is impacting my ability to make other PRs. Temporarily deleting it and I'll make a new PR later today.

@WhitWaldo WhitWaldo closed this Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/jobs waiting-on-code-pr The code PR needs to be merged before the docs are updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants