Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new resource-scoped “jobs” plugin to @databricks/appkit, following the existing “files” plugin pattern: jobs are discovered from environment variables at startup and exposed via a keyed accessor API, with HTTP routes for triggering and monitoring runs.
Changes:
- Introduces
plugins/jobs(manifest, defaults, params mapping, types, plugin implementation, and extensive tests). - Adds
connectors/jobswith telemetry + cancellation support and updates exports/docs to surface the new plugin and types. - Updates templates and generated API docs/sidebars to include the new Jobs plugin.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| template/appkit.plugins.json | Adds “jobs” to plugin template metadata + resource description. |
| pnpm-lock.yaml | Locks new dependency (zod@4.3.6) and related lockfile updates. |
| packages/appkit/src/plugins/jobs/types.ts | Public types for Jobs plugin API/config (JobAPI/JobHandle/IJobsConfig). |
| packages/appkit/src/plugins/jobs/plugin.ts | Core JobsPlugin: env discovery, dynamic resource requirements, API methods, HTTP routes. |
| packages/appkit/src/plugins/jobs/params.ts | TaskType-based param mapping into SDK request fields. |
| packages/appkit/src/plugins/jobs/defaults.ts | Execution defaults for read/write/stream operations. |
| packages/appkit/src/plugins/jobs/manifest.json | Plugin manifest + config schema + baseline resource definition. |
| packages/appkit/src/plugins/jobs/index.ts | Barrel exports for Jobs plugin/types. |
| packages/appkit/src/plugins/jobs/tests/plugin.test.ts | Comprehensive unit tests for discovery, API, routes, and validation. |
| packages/appkit/src/plugins/index.ts | Exposes jobs from the plugins barrel. |
| packages/appkit/src/index.ts | Exposes jobs and related public types/configs from package root. |
| packages/appkit/src/connectors/jobs/client.ts | JobsConnector SDK wrapper with telemetry instrumentation + limit handling. |
| packages/appkit/src/connectors/jobs/types.ts | Connector config type (timeout/telemetry). |
| packages/appkit/src/connectors/jobs/index.ts | Connector barrel exports. |
| packages/appkit/src/connectors/index.ts | Exposes jobs connector from connectors barrel. |
| packages/appkit/package.json | Adds zod dependency for runtime param schemas + JSON schema generation. |
| docs/docs/api/appkit/typedoc-sidebar.ts | Adds new Jobs docs entries to sidebar. |
| docs/docs/api/appkit/*.md | Adds generated API docs pages for Jobs plugin types. |
| docs/docs/api/appkit/index.md | Adds Jobs-related types to API index list. |
| docs/docs/api/appkit/Interface.BasePluginConfig.md | Updates “Extended by” list to include IJobsConfig. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
How it is different from @keugenek PR here: #221? If you based on top of Evgenii's work, maybe it's worth to recognize his contribution? What I'm thinking is either:
WDYT? |
I got stuck trying to update his PR because it was opened from a fork, it's noted in the PR description:
So, I couldn't get a clean CI to merge it, and if I merged to It's also marked as a comment on the original PR |
It wouldn't trigger a release currently (finalizing releases is manual) so there shouldn't be a problem with that 👍 but I'd wait until your PR (on top of this one) is ready to merge, and I'd merge both one after another. What do you think? |
493146a to
791b146
Compare
040f7bd to
d8133b9
Compare
MarioCadenas
left a comment
There was a problem hiding this comment.
I added some comments, but I'm actually wondering if this should just be a "jobs" plugin or if we should extend this and make it support both jobs and pipelines tbh 😅
| */ | ||
| export const jobs = toPlugin(JobsPlugin); | ||
|
|
||
| export { JobsPlugin }; |
There was a problem hiding this comment.
why do we need to export this? tests?
There was a problem hiding this comment.
correct. I should probably add an @internal here.
it makes the tests a bit easier to write by reaching the statics directly
I can remove it though, just felt the tradeoff was worth it
| return result.ok ? result : errorResult(result.status); | ||
| }, | ||
|
|
||
| async *runAndWait( |
There was a problem hiding this comment.
can runAndWait be simplified? maybe we can extract some parts?
|
a few more things I noticed going through the core package: 1. in self._readSettings(["jobs:listRuns", jobKey, options ?? {}])when 2. in notebook_params: Object.fromEntries(
Object.entries(params).map(([k, v]) => [k, String(v)]),
),if someone passes an object or array through (e.g. with a loose zod schema like 3. polling in I know I already asked if we can simplify |
pkosiec
left a comment
There was a problem hiding this comment.
Just a few comments 👍 Nice work!
c379239 to
b6cd0cf
Compare
Jobs are configured as named resources (DATABRICKS_JOB_<KEY> env vars)
and discovered at startup, following the files plugin pattern.
API is scoped to configured jobs:
appkit.jobs('etl').runNow()
appkit.jobs('etl').runNowAndWait()
appkit.jobs('etl').lastRun()
appkit.jobs('etl').listRuns()
appkit.jobs('etl').asUser(req).runNow()
Single-job shorthand via DATABRICKS_JOB_ID env var.
Supports OBO access via asUser(req).
Co-authored-by: Isaac
Signed-off-by: Evgenii Kniazev <evgenii.kniazev@databricks.com>
HTTP and SDK surface: - injectRoutes: POST /:jobKey/run (JSON or SSE via ?stream=true), GET /:jobKey/runs, GET /:jobKey/runs/:runId, GET /:jobKey/status, DELETE /:jobKey/runs/:runId - runAndWait streams JobRunStatus updates with abortable polling, terminal-state detection, and exponential backoff with jitter - _parseRunParams eagerly validates request bodies so streaming requests get a clean 400 instead of a generic SSE error - Guard against raw params on jobs with neither a schema nor a taskType - Execution interceptors wired via _readSettings / _writeSettings; stream defaults for SSE - Tests covering discovery, validation, routes, error propagation, OBO Cleanup and polish: - Mark JobsPlugin export @internal (test-only access) - Shorten manifest job ID field description; env var is already declared via the resource field's env key - Remove install-appkit-tarball skill superseded upstream by install-appkit-artifact Playground + template: - Buffer incomplete SSE lines across reads so data: events split across chunks are not dropped - Cap stream log at 500 entries to prevent unbounded growth - Use counter-based keys on each log entry to avoid React key collisions Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
65bcd39 to
e5e2427
Compare
Important
to maintain contribution history this PR should be rebased when merged
(don't merge it if it has more than 2 commits)
Summary
Resource-scoped jobs plugin following the files plugin pattern. Jobs are configured as named resources discovered from environment variables at startup.
Design
DATABRICKS_JOB_<KEY>env vars (e.g.DATABRICKS_JOB_ETL=123)DATABRICKS_JOB_IDmaps to the"default"keyCAN_MANAGE_RUNpermissiondatabricks apps init --features jobsAPI
Files changed
plugins/jobs/manifest.json— declares job resource withCAN_MANAGE_RUNpermissionplugins/jobs/types.ts—JobAPI,JobHandle,JobsExport,IJobsConfigtypesplugins/jobs/plugin.ts—JobsPluginwithdiscoverJobs(),getResourceRequirements(), resource-scopedcreateJobAPI()plugins/jobs/index.ts— barrel exportsconnectors/jobs/client.ts—listRunsnow respectslimitparameterplugins/jobs/tests/plugin.test.ts— 32 tests covering discovery, resource requirements, exports, OBO, multi-job, and auto-fillDocumentation safety checklist
demo
lakeflow-jobs.mp4