Skip to content

feat: add jobs #688

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

Draft
wants to merge 31 commits into
base: main
Choose a base branch
from
Draft

feat: add jobs #688

wants to merge 31 commits into from

Conversation

atrauzzi
Copy link

Description

Added initial support for jobs.

Issue reference

Closes: #667

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

@atrauzzi atrauzzi changed the title feat: feat: add jobs Mar 15, 2025
@WhitWaldo WhitWaldo added this to the v3.6.0 milestone Mar 16, 2025
@WhitWaldo
Copy link
Contributor

@atrauzzi This runs a linter (for now) as part of the build pipeline and failures will stop the build. Would you mind fixing the lint errors locally and pushing a commit with those changes as well so we can run the checks?

Copy link
Contributor

@WhitWaldo WhitWaldo left a comment

Choose a reason for hiding this comment

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

Excellent start - still a bit to go, but it's a good foundation.

import { TypeDaprJobsCallback } from "../../types/DaprJobsCallback.type";

export default interface IServerJobs {
listen(jobName: string, callback: TypeDaprJobsCallback): void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here - I don't know that the server will ever pass along a job invocation without a payload (e.g. though of course it might be empty).

Copy link
Author

Choose a reason for hiding this comment

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

See above 🙂


type EveryPeriod = `@every ${string}`;

// note: This can get crazy, more than TS can really handle.
Copy link
Contributor

Choose a reason for hiding this comment

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

You might consider taking a look at how I built this out in the .NET SDK to accommodate each variation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Most importantly - there are different sets of arguments that need to be provided based on this schedule and the other arguments used.

Copy link
Author

Choose a reason for hiding this comment

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

Mind elaborating on the different sets of arguments? I expanded things a fair bit in a recent push, maybe you wanna look at that and if you want, we can discuss here or in discord 🙂

Copy link
Contributor

@WhitWaldo WhitWaldo left a comment

Choose a reason for hiding this comment

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

Coming along - note about the regexps though

Copy link
Contributor

@WhitWaldo WhitWaldo left a comment

Choose a reason for hiding this comment

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

More thoughts

@@ -11,17 +11,17 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { JobSchedule } from "../../types/jobs/JobSchedule.type";
import { Schedule } from "../../types/jobs/JobSchedule.type";
Copy link
Contributor

Choose a reason for hiding this comment

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

Especially if the file only contains a single type, the type name should match the file name.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, agreed.

listen(jobName: string, callback: TypeDaprJobsCallback): void;

listen<DataType>(jobName: string, callback: TypeDaprJobsCallback<DataType>): void;
listen(jobName: string, callback: (data: any) => unknown): void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this return an unknown? While the callback needn't necessarily return anything, the SDK does need to know that it was handled without error so it can send back a 200 OK.

Copy link
Author

Choose a reason for hiding this comment

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

That's the callback that users provide. I mark it as unknown so that it can be optionally sync or async.

Should we be imposing some kind of contract for the implementation to return, or just assume handling was successful if no exception is thrown?

atrauzzi added a commit to atrauzzi/dapr-js-sdk that referenced this pull request Mar 22, 2025
 - Addresses
   - dapr#688 (comment)
   - dapr#688 (comment)

Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Comment on lines 26 to 30
export type SecondOrMinute = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21 | 22 | 23 | 24 | 25 | 26 | 27 | 28 | 29 | 30 | 31 | 32 | 33 | 34 | 35 | 36 | 37 | 38 | 39 | 40 | 41 | 42 | 43 | 44 | 45 | 46 | 47 | 48 | 49 | 50 | 51 | 52 | 53 | 54 | 55 | 56 | 57 | 58 | 59;
export type Hour = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21 | 22 | 23;
export type DayOfMonth = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21 | 22 | 23 | 24 | 25 | 26 | 27 | 28 | 29 | 30 | 31;
export type DayOfWeekNumber = 0 | 1 | 2 | 3 | 4 | 5 | 6;
export type MonthNumber = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12;

Choose a reason for hiding this comment

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

This is a bit of a NIT, but theres a different way of adding type for these situations, which ive used before.
You would have first a type that would help you build such types:

type BuildNumberRange<T extends number, U extends number, R extends number[] = []> = R['length'] extends U
  ? R[number]
  : BuildNumberRange<T, U, [...R, R['length'] & number]>;

And then you can have your types like this:

export type SecondOrMinute = BuildNumberRange<0, 59>
export type Hour  = BuildNumberRange<0, 23>
export type DayOfMonth = BuildNumberRange<1, 31>
export type DayOfWeekNumber = BuildNumberRange<0, 6>
export type MonthNumber = BuildNumberRange<1, 12>

Just an idea. I realise the BuildNumberRange type is quite hard to reason with for new comers, but its the type of "set it and forget it".

Copy link
Author

Choose a reason for hiding this comment

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

That's pretty slick.

@joebowbeer
Copy link
Contributor

joebowbeer commented Apr 19, 2025

== APP ==     Error: 14 UNAVAILABLE: connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:50001: connect: connection refused"
== APP ==         at callErrorFromStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/call.ts:82:17)
== APP ==         at Object.onReceiveStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client.ts:360:55)
== APP ==         at Object.onReceiveStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client-interceptors.ts:458:34)
== APP ==         at Object.onReceiveStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client-interceptors.ts:419:48)
== APP ==         at /home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/resolving-call.ts:163:24
== APP ==         at processTicksAndRejections (node:internal/process/task_queues:78:11)
== APP ==     for call at
== APP ==         at ServiceClientImpl.makeUnaryRequest (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client.ts:325:42)
== APP ==         at ServiceClientImpl.invokeService (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/make-client.ts:189:15)
== APP ==         at /home/runner/work/js-sdk/js-sdk/src/implementation/Client/GRPCClient/invoker.ts:64:14
== APP ==         at new Promise (<anonymous>)
== APP ==         at GRPCClientInvoker.invoke (/home/runner/work/js-sdk/js-sdk/src/implementation/Client/GRPCClient/invoker.ts:63:12)
== APP ==         at Object.<anonymous> (/home/runner/work/js-sdk/js-sdk/test/e2e/grpc/server.test.ts:78:21) {
== APP ==       code: 14,
== APP ==       details: 'connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:50001: connect: connection refused"',
== APP ==       metadata: Metadata {
== APP ==         internalRepr: Map(2) { 'traceparent' => [Array], 'grpc-trace-bin' => [Array] },
== APP ==         options: {}
== APP ==       }
== APP ==     }
== APP == 
== APP ==       at Object.<anonymous> (test/e2e/grpc/server.test.ts:81:17)

@WhitWaldo
Copy link
Contributor

== APP ==     Error: 14 UNAVAILABLE: connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:50001: connect: connection refused"
== APP ==         at callErrorFromStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/call.ts:82:17)
== APP ==         at Object.onReceiveStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client.ts:360:55)
== APP ==         at Object.onReceiveStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client-interceptors.ts:458:34)
== APP ==         at Object.onReceiveStatus (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client-interceptors.ts:419:48)
== APP ==         at /home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/resolving-call.ts:163:24
== APP ==         at processTicksAndRejections (node:internal/process/task_queues:78:11)
== APP ==     for call at
== APP ==         at ServiceClientImpl.makeUnaryRequest (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/client.ts:325:42)
== APP ==         at ServiceClientImpl.invokeService (/home/runner/work/js-sdk/js-sdk/node_modules/@grpc/grpc-js/src/make-client.ts:189:15)
== APP ==         at /home/runner/work/js-sdk/js-sdk/src/implementation/Client/GRPCClient/invoker.ts:64:14
== APP ==         at new Promise (<anonymous>)
== APP ==         at GRPCClientInvoker.invoke (/home/runner/work/js-sdk/js-sdk/src/implementation/Client/GRPCClient/invoker.ts:63:12)
== APP ==         at Object.<anonymous> (/home/runner/work/js-sdk/js-sdk/test/e2e/grpc/server.test.ts:78:21) {
== APP ==       code: 14,
== APP ==       details: 'connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:50001: connect: connection refused"',
== APP ==       metadata: Metadata {
== APP ==         internalRepr: Map(2) { 'traceparent' => [Array], 'grpc-trace-bin' => [Array] },
== APP ==         options: {}
== APP ==       }
== APP ==     }
== APP == 
== APP ==       at Object.<anonymous> (test/e2e/grpc/server.test.ts:81:17)

What's this in reference to?

@joebowbeer
Copy link
Contributor

joebowbeer commented Apr 19, 2025

What's this in reference to?

@WhitWaldo This is the failure from the test run of the previous commit.

However, I don't think it sheds much light. I think the interesting failure is in the scheduler's (Testcontainer) log:

"failed to invoke schedule app job:
  error returned from app channel while sending triggered job to app:
    Post \"http://127.0.0.1:8070/job/test\": EOF"

I suspect this 127.0.0.1 is wrong.

@atrauzzi
Copy link
Author

Alright, so a small bit of progress on connectivity between the dapr daemon and the scheduler, but sadly not quite there with things yet.

I'm now running into a timeout scenario where it seems like any attempt to use the client.

dapr-js-sdk testcontainers jest timeout.txt

The code seems to hang when I try to create the job. @WhitWaldo, maybe you can spot something?

.withTmpFs({
"/data": "rw",
})
// note: Because dapr containers don't have `sh` or `bash` inside, this is kind of the best health check.
Copy link
Contributor

Choose a reason for hiding this comment

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

Two comments:

  1. Wait.forHttp(path, port) works for me, and is what Dapr's Testcontainer for Java does:

https://github.com/dapr/java-sdk/blob/ab8e41111de17ea5bc47b9d94dbab6fe30577590/testcontainers-dapr/src/main/java/io/dapr/testcontainers/DaprContainer.java#L60

  1. The timeout should be much longer, e.g., 120_000

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest something like the following for the wait strategy

  .withWaitStrategy(Wait.forHttp("/v1.0/healthz/outbound", DAPRD_HTTP_PORT)
    .forStatusCodeMatching((statusCode) => statusCode >= 200 && statusCode <= 399))
  .withStartupTimeout(120_000)

Copy link
Author

Choose a reason for hiding this comment

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

This has proven to be the trickiest, but also avoidable if I stick with my prior approach, waiting for console output. For some reason that endpoint never seems to ready up.

Copy link

codecov bot commented Jul 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (d1bf38a) to head (74a3f99).
Report is 31 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #688   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            6         6           
  Branches         1         1           
=========================================
  Hits             6         6           

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

atrauzzi added 6 commits July 12, 2025 15:49
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
atrauzzi and others added 21 commits July 12, 2025 15:49
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
(...make Jack a dull boy)

Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
 - Addresses
   - dapr#688 (comment)
   - dapr#688 (comment)

Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
* Removing prettifier from the build pipeline as it's an unnecessary step during a build operation.
* Version number updated in package-lock.json

Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Removed references to Workflow components and replaced with "dapr" for the name in the HTTP client.

Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Removes stale bot + adds me to repo owners for bot actions

Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Joe Bowbeer <joe.bowbeer@gmail.com>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
atrauzzi added 2 commits July 12, 2025 15:53
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>

# Conflicts:
#	package-lock.json
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
@atrauzzi atrauzzi requested a review from WhitWaldo July 13, 2025 00:27
atrauzzi added 2 commits July 12, 2025 19:28
Signed-off-by: Alexander Trauzzi <acj@trauzzi.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Jobs] Add support for Jobs building block
4 participants