Exposes FailurePolicy to the Jobs api #737

acroca · 2025-06-18T14:47:37Z

Description

dapr/dapr#8785 has been merged into master. That PR exposes FailurePolicy field in the Jobs API

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation

Signed-off-by: Albert Callarisa <[email protected]>

cicoyle

Thanks for the name change to jobs for consistency across dapr repos - I've been wanting to update that for a while now 🙌🏻

JoshVanL · 2025-06-19T14:26:59Z

client/jobs.go

+	Name          string
+	Schedule      string // Optional
+	Repeats       uint32 // Optional
+	DueTime       string // Optional
+	TTL           string // Optional
+	Data          *anypb.Any


Optional fields should be pointers.

JoshVanL · 2025-06-19T14:27:20Z

client/jobs.go

+	if job.Schedule != "" {
+		jobRequest.Schedule = &job.Schedule
+	}
+
+	if job.Repeats != 0 {
+		jobRequest.Repeats = &job.Repeats
+	}
+
+	if job.DueTime != "" {
+		jobRequest.DueTime = &job.DueTime
+	}
+
+	if job.TTL != "" {
+		jobRequest.Ttl = &job.TTL
+	}
+
+	if job.FailurePolicy != nil {
+		jobRequest.FailurePolicy = job.FailurePolicy.GetPBFailurePolicy()


Assigned pointers to pointers. We shouldn't empty string match.

JoshVanL · 2025-06-19T14:27:29Z

client/jobs.go

+	resp, err := c.protoClient.GetJobAlpha1(ctx, &runtimepb.GetJobRequest{
+		Name: name,
+	})
+	log.Println(resp)


JoshVanL · 2025-06-19T14:27:55Z

client/jobs.go

+func (f *JobFailurePolicyConstant) GetPBFailurePolicy() *commonpb.JobFailurePolicy {
+	policy := &commonpb.JobFailurePolicy{
+		Policy: &commonpb.JobFailurePolicy_Constant{
+			Constant: &commonpb.JobFailurePolicyConstant{},
+		},
+	}
+	if f.maxRetries != nil {
+		policy.Policy.(*commonpb.JobFailurePolicy_Constant).Constant.MaxRetries = f.maxRetries
+	}
+	if f.interval != nil {
+		policy.Policy.(*commonpb.JobFailurePolicy_Constant).Constant.Interval = &durationpb.Duration{Seconds: int64(f.interval.Seconds())}
+	}
+	return policy
+}


Why and what is this?

JobFailurePolicyConstant is the exposed type the SDK users can use to assign FailurePolicies. This function then converts this exposed type into the corresponding policy from the commonpb package.

JoshVanL · 2025-06-19T14:28:50Z

client/jobs.go

+func (f *JobFailurePolicyConstant) GetPBFailurePolicy() *commonpb.JobFailurePolicy {
+	policy := &commonpb.JobFailurePolicy{
+		Policy: &commonpb.JobFailurePolicy_Constant{
+			Constant: &commonpb.JobFailurePolicyConstant{},
+		},
+	}
+	if f.maxRetries != nil {
+		policy.Policy.(*commonpb.JobFailurePolicy_Constant).Constant.MaxRetries = f.maxRetries
+	}
+	if f.interval != nil {
+		policy.Policy.(*commonpb.JobFailurePolicy_Constant).Constant.Interval = &durationpb.Duration{Seconds: int64(f.interval.Seconds())}
+	}


Can this not be built in a better way to avoid type casting the proto pointer?

Also: - Refactor how exposed type is converted to proto type - Remove unnecesary log - Input validation for job api calls Signed-off-by: Albert Callarisa <[email protected]>

client/utils.go

go.mod

mikeee · 2025-06-19T17:48:23Z

examples/go.sum

Examples folder needs a go mod tidy as v1.15.5 is still vendored it seems

mikeee · 2025-06-19T17:50:58Z

client/client_test.go

+		dueTime           = "10s"
+		repeats    uint32 = 4
+		ttl               = "10s"
+		maxRetries        = uint32(4)


Suggested change

maxRetries = uint32(4)

maxRetries uint32 = 4

mikeee · 2025-06-19T17:56:41Z

client/jobs.go

+	}
+}
+
+func NewFailurePolicyConstant(maxRetries *uint32, interval *time.Duration) FailurePolicy {


This function signature doesn't appear idiomatic to me... having to pass an address of an unsigned in and again the address of a duration
I would have liked to see the values passed directly e.g. NewFailurePolicyConstant(4, time.Second * 5)

I'll actually remove those 'constructor' functions, as they are now unnecessary (those types were not exposed previously). I think it'll cleaner just to provide the appropriate struct directly, like the following:

&Job{ Name: "name", Schedule: &schedule, Repeats: &repeats, DueTime: &dueTime, TTL: &ttl, Data: nil, FailurePolicy: &JobFailurePolicyConstant{ maxRetries: &maxRetries, interval: &interval, }, }

What do you think?

I missed this! I posted #737 (comment)

I think the functional options pattern is more idiomatic whether applied directly on the Job or as an optional overload to the ScheduleJobAlpha1 method

client/jobs.go

mikeee · 2025-06-19T18:16:18Z

examples/jobs/main.go

+	maxRetries := uint32(4)
+	interval := time.Second * 3
 	job := daprc.Job{
 		Name:     "prod-db-backup",
 		Schedule: "@every 1s",
 		Repeats:  10,
 		Data: &anypb.Any{
 			Value: jobData,
 		},
+		FailurePolicy: daprc.NewFailurePolicyConstant(&maxRetries, &interval),
 	}


I think from this example it's clear that using the addresses of an int or duration adds unnecessary faff in my opinion

Signed-off-by: Albert Callarisa <[email protected]>

We can pass the right types directly when building jobs, there's no need for a constructor. The constructor was used previously when the types were private, but now they are exposed so the constructor is unnecessary. Signed-off-by: Albert Callarisa <[email protected]>

Signed-off-by: Albert Callarisa <[email protected]>

mikeee

Looking good, would be good to see some test assertions for those functional options just to cover any future upstream changes for this Alpha api

Signed-off-by: Albert Callarisa <[email protected]>

acroca · 2025-07-04T05:33:05Z

@mikeee What do you think now?

codecov · 2025-07-08T10:23:42Z

Codecov Report

Attention: Patch coverage is 67.62590% with 45 lines in your changes missing coverage. Please review.

Project coverage is 57.18%. Comparing base (6c59092) to head (62f9b56).
Report is 26 commits behind head on main.

Files with missing lines	Patch %	Lines
client/jobs.go	67.62%	34 Missing and 11 partials ⚠️

❗ There is a different number of reports uploaded between BASE (6c59092) and HEAD (62f9b56). Click for more details.

HEAD has 3 uploads less than BASE

Flag BASE (6c59092) HEAD (62f9b56)

4 1

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #737      +/-   ##
==========================================
- Coverage   62.52%   57.18%   -5.35%     
==========================================
  Files          56       58       +2     
  Lines        4139     4428     +289     
==========================================
- Hits         2588     2532      -56     
- Misses       1425     1761     +336     
- Partials      126      135       +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mikeee

@acroca Looking good, I think the linter isn't happy with the way you're accessing the proto fields but aside from that it looks fine to me.

Could you please address two things also:

Deps issue whereby the JobFailurePolicy is not found, assumign this is a result of whichever commit hash you've used
Update the workflow (https://github.com/dapr/go-sdk/blob/main/.github/workflows/validate_examples.yaml) rename the 'dist-scheduler' item to 'jobs'

Signed-off-by: Albert Callarisa <[email protected]>

acroca · 2025-07-11T08:27:25Z

@mikeee better now? The failing test seems to be github related, re-running it should work (works on my machine ™)

mikeee

Lgtm- the tests are flaky when initialising dapr.

acroca · 2025-07-14T05:54:00Z

Cool, what's left to merge this PR?

Exposes FailurePolicy to the Jobs api

927c901

Signed-off-by: Albert Callarisa <[email protected]>

acroca requested review from a team as code owners June 18, 2025 14:47

Added documentation for the jobs api

f6d10d3

Signed-off-by: Albert Callarisa <[email protected]>

cicoyle approved these changes Jun 18, 2025

View reviewed changes

This was referenced Jun 19, 2025

Add support for Jobs FailurePolicy dapr/dotnet-sdk#1557

Closed

Add support for Jobs FailurePolicy dapr/java-sdk#1429

Open

Add support for Jobs FailurePolicy dapr/js-sdk#704

Open

JoshVanL suggested changes Jun 19, 2025

View reviewed changes

acroca mentioned this pull request Jun 19, 2025

[FEATURE REQUEST] Add support for Jobs FailurePolicy dapr/python-sdk#808

Open

acroca requested a review from JoshVanL June 19, 2025 16:04

acroca force-pushed the expose-jobs-failure-policy branch from c33b552 to ae9a14e Compare June 19, 2025 16:21

Use pointers for optional fields

88a5872

Also: - Refactor how exposed type is converted to proto type - Remove unnecesary log - Input validation for job api calls Signed-off-by: Albert Callarisa <[email protected]>

acroca force-pushed the expose-jobs-failure-policy branch from ae9a14e to 88a5872 Compare June 19, 2025 16:22

mikeee requested changes Jun 19, 2025

View reviewed changes

acroca added 5 commits June 20, 2025 09:35

clearer variable type

ddc23de

Signed-off-by: Albert Callarisa <[email protected]>

Remove unnecessary helper function

3eb02a4

Signed-off-by: Albert Callarisa <[email protected]>

Use dapr/kit for inline pointers. Adjusted docs and examples.

3cddda3

Signed-off-by: Albert Callarisa <[email protected]>

Use functional options to build job objects

c7bb043

Signed-off-by: Albert Callarisa <[email protected]>

mikeee reviewed Jun 21, 2025

View reviewed changes

Add unit tests for the job builder

c0cad3e

Signed-off-by: Albert Callarisa <[email protected]>

acroca requested a review from mikeee June 30, 2025 06:14

acroca mentioned this pull request Jul 8, 2025

REQUEST: New membership for acroca dapr/community#629

Closed

4 tasks

mikeee requested changes Jul 8, 2025

View reviewed changes

acroca added 3 commits July 8, 2025 12:50

fix examples validator

3d519b2

Signed-off-by: Albert Callarisa <[email protected]>

Fix lint errors

628dcc1

Signed-off-by: Albert Callarisa <[email protected]>

fix examples dependencies

c349d4a

Signed-off-by: Albert Callarisa <[email protected]>

acroca requested a review from mikeee July 8, 2025 11:08

acroca added 2 commits July 8, 2025 14:38

go mod tidy

b92f20f

Signed-off-by: Albert Callarisa <[email protected]>

Fix job example validation

62f9b56

Signed-off-by: Albert Callarisa <[email protected]>

mikeee approved these changes Jul 11, 2025

View reviewed changes

yaron2 merged commit 0c36331 into dapr:main Jul 14, 2025
31 of 33 checks passed

acroca deleted the expose-jobs-failure-policy branch July 14, 2025 13:56

Exposes FailurePolicy to the Jobs api #737

Exposes FailurePolicy to the Jobs api #737

Uh oh!

Conversation

acroca commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

cicoyle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikeee left a comment

Choose a reason for hiding this comment

Uh oh!

acroca commented Jul 4, 2025

Uh oh!

codecov bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mikeee left a comment

Choose a reason for hiding this comment

Uh oh!

acroca commented Jul 11, 2025

Uh oh!

mikeee left a comment

Choose a reason for hiding this comment

Uh oh!

acroca commented Jul 14, 2025

Uh oh!

Uh oh!

Uh oh!

acroca commented Jun 18, 2025 •

edited

Loading

codecov bot commented Jul 8, 2025 •

edited

Loading