Repository navigation
chore(ecc-utils-ga4gh-tes): tes migration - #516
salihudickson wants to merge 18 commits into
Conversation
Reviewer's GuideThis PR performs a major migration of @elixir-cloud/tes to the v2 providers architecture, replacing legacy TES APIs and components with typed provider-backed task creation, listing, detail, and cancellation workflows; it also adds Tailwind/design-system styling, typed events, comprehensive tests, release integration, documentation, and demos. Sequence diagram for creating a TES tasksequenceDiagram
participant User
participant RunCreate as ECCClientGa4ghTesRunCreate
participant Provider as TesProvider
participant TES as GA4GH_TES_API
User->>RunCreate: Submit task form
RunCreate->>RunCreate: validate()
RunCreate->>RunCreate: buildRequest()
RunCreate->>Provider: createTask(request)
Provider->>TES: POST /tasks
TES-->>Provider: TaskId
Provider-->>RunCreate: TaskId
RunCreate-->>User: ecc-run-submitted
Sequence diagram for listing and cancelling TES taskssequenceDiagram
participant User
participant Runs as ECCClientGa4ghTesRuns
participant Provider as TesProvider
participant TES as GA4GH_TES_API
Runs->>Provider: listTasks(params)
Provider->>TES: GET /tasks
TES-->>Provider: TaskListResponse
Provider-->>Runs: TaskListResponse
Runs-->>User: Render tasks and pagination
User->>Runs: Delete active task
Runs->>Provider: cancelTask(runId)
Provider->>TES: POST /tasks/{taskId}:cancel
TES-->>Provider: Success
Provider-->>Runs: void
Runs->>Provider: listTasks(params)
Provider-->>Runs: Updated task list
Sequence diagram for loading TES task detailssequenceDiagram
participant User
participant Run as ECCClientGa4ghTesRun
participant Provider as TesProvider
participant TES as GA4GH_TES_API
User->>Run: Set runId
Run->>Provider: getTask(runId, FULL)
Provider->>TES: GET /tasks/{taskId}?view=FULL
TES-->>Provider: Task
Provider-->>Run: Task
Run-->>User: Render overview, executors, inputs/outputs, logs
Run-->>User: ecc-run-log-changed
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved request races can overwrite current data and cancel the wrong task, and the mock API cannot start because its server implementation is missing.
9 open issues
Prevent stale task responses from replacing the active task · New Guard list responses against stale requests · New Provide the missing mock server implementation · New Preserve command argument lines verbatim · New Forward accessible labels to native inputs · New Clear cached pagination tokens after refresh · New Preserve literal state types with as const · New Document the renamed creation element and import · New Add the missing TES documentation pages · New
What changed in this PR
Migrates @elixir-cloud/tes to the v2 provider architecture described in #443, aligning TES integration with the other client packages.
Changes:
- Introduces a TES provider interface, REST implementation, API models, and event exports.
- Replaces legacy components with task listing, detail, and creation views using shared design primitives and Tailwind styling.
- Updates package exports, build tooling, demos, documentation navigation, and provider-backed listing tests.
| File | Description |
|---|---|
packages/mock-api/package.json |
Adds mock-service package metadata and start scripts. |
packages/ecc-client-ga4gh-tes/web-test-runner.config.mjs |
Sets the test transformation target to ES2020. |
packages/ecc-client-ga4gh-tes/tsconfig.json |
Switches to Node16 module resolution. |
packages/ecc-client-ga4gh-tes/src/tailwind.css |
Defines Tailwind theme mappings and variants. |
packages/ecc-client-ga4gh-tes/src/providers/tes-provider.ts |
Defines TES models and provider operations. |
packages/ecc-client-ga4gh-tes/src/providers/rest-tes-provider.ts |
Implements REST-backed TES operations. |
packages/ecc-client-ga4gh-tes/src/providers/index.ts |
Exports provider APIs. |
packages/ecc-client-ga4gh-tes/src/global.ts |
Adds shared light and dark theme tokens. |
packages/ecc-client-ga4gh-tes/src/events/index.ts |
Exports component event types. |
packages/ecc-client-ga4gh-tes/src/events/ecc-runs-selected.ts |
Declares the task-selection event type. |
packages/ecc-client-ga4gh-tes/src/events/ecc-runs-changed.ts |
Declares the task-list change event type. |
packages/ecc-client-ga4gh-tes/src/events/ecc-run-submitted.ts |
Declares the task-submission event type. |
packages/ecc-client-ga4gh-tes/src/events/ecc-run-log-changed.ts |
Declares the task-data change event type. |
packages/ecc-client-ga4gh-tes/src/events/ecc-run-create-reset.ts |
Declares the creation-form reset event type. |
packages/ecc-client-ga4gh-tes/src/events/ecc-run-create-failed.ts |
Declares the task-creation failure event type. |
packages/ecc-client-ga4gh-tes/src/components/runs/runs.ts |
Rebuilds listing with providers, filters, pagination, and cancellation. |
packages/ecc-client-ga4gh-tes/src/components/runs/runs.test.ts |
Tests provider-backed listing, selection, and cancellation. |
packages/ecc-client-ga4gh-tes/src/components/run/run.ts |
Adds task details, logs, and cancellation. |
packages/ecc-client-ga4gh-tes/src/components/run/index.ts |
Registers the task-detail element. |
packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts |
Adds the provider-backed task-creation form. |
packages/ecc-client-ga4gh-tes/src/components/run-create/index.ts |
Registers the new task-creation element. |
packages/ecc-client-ga4gh-tes/src/components/index.ts |
Loads all migrated component registrations. |
packages/ecc-client-ga4gh-tes/src/components/create-run/index.ts |
Removes the legacy creation-element registration. |
packages/ecc-client-ga4gh-tes/src/components/create-run/create-runs.styles.ts |
Removes the legacy style placeholder. |
packages/ecc-client-ga4gh-tes/src/components/create-run/create-run.ts |
Removes the legacy creation component. |
packages/ecc-client-ga4gh-tes/src/components/create-run/create-run.test.ts |
Legacy creation-test file; no diff supplied. |
packages/ecc-client-ga4gh-tes/src/API/Task/tesGet.ts |
Removes API helpers superseded by providers. |
packages/ecc-client-ga4gh-tes/src/API/Task/README.md |
Removes legacy helper documentation. |
packages/ecc-client-ga4gh-tes/src/API/index.ts |
Removes the legacy API entry point. |
packages/ecc-client-ga4gh-tes/package.json |
Updates release version, exports, dependencies, and scripts. |
packages/ecc-client-ga4gh-tes/demo/runs/index.html |
Updates the listing demo and detail navigation. |
packages/ecc-client-ga4gh-tes/demo/run/index.html |
Adds a task-detail demo. |
packages/ecc-client-ga4gh-tes/demo/run-create/index.html |
Adds a task-creation demo. |
packages/ecc-client-ga4gh-tes/demo/index.html |
Updates links to the migrated demos. |
packages/ecc-client-ga4gh-tes/demo/create-run/index.html |
Removes the legacy creation demo. |
packages/ecc-client-ga4gh-tes/assets/delete.svg |
Removes the legacy delete icon. |
packages/ecc-client-ga4gh-tes/.gitignore |
Excludes generated styles and build artifacts. |
packages/ecc-client-ga4gh-tes/.eslintrc |
Adjusts package lint rules. |
packages/ecc-client-ga4gh-tes/.eslintignore |
Excludes generated styling files from linting. |
package-lock.json |
Records updated TES dependencies. |
apps/documentation/pages/docs/tes/components/_meta.json |
Adds TES component navigation labels. |
apps/documentation/pages/docs/tes/_meta.json |
Defines TES documentation navigation. |
apps/documentation/pages/docs/_meta.json |
Adds the TES documentation section. |
.changeset/tes-providers-architecture.md |
Records the migration as a major package change. |
Review effort: Balanced
| <ecc-utils-design-input | ||
| .value=${current} | ||
| type=${options.type ?? "text"} | ||
| placeholder=${options.placeholder ?? ""} | ||
| class="h-10" |
| "introduction": "Introduction", | ||
| "providers": "Providers", | ||
| "components": "Components" |
cb56922 to
f73f42b
Compare
Documentation build overview
51 files changed ·
|
6e70aad to
c30578d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #516 +/- ##
==========================================
+ Coverage 70.89% 76.47% +5.58%
==========================================
Files 23 30 +7
Lines 5638 7287 +1649
Branches 190 347 +157
==========================================
+ Hits 3997 5573 +1576
- Misses 1636 1707 +71
- Partials 5 7 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
f734385 to
26fa6dc
Compare
There was a problem hiding this comment.
Hey - I've found 11 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts" line_range="733-736" />
<code_context>
+ Reset
+ </ecc-utils-design-button>
+ <ecc-utils-design-button
+ type="submit"
+ ?disabled=${this.submitting}
+ @click=${this.handleSubmit}
+ >
+ ${this.submitting ? "Submitting..." : "Submit Task"}
+ </ecc-utils-design-button>
</code_context>
<issue_to_address>
**Task submissions run twice**
When a user clicks Submit Task or another design button in the create form, the design button’s internal native `<button>` has no `type`, so it defaults to `submit` inside this form; the host’s `type` attribute does not set the internal button’s type. The submit button therefore calls `handleSubmit` from its click listener and again from the form’s submit handler, creating two tasks; Add controls also submit the form.
Make the design button forward its `type` property to the internal button, and handle submission through only one of the click or form-submit paths.
Also at `packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts:367-371`.
</issue_to_address>
### Comment 2
<location path="packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts" line_range="277" />
<code_context>
+---
+"@elixir-cloud/tes": major
+---
</code_context>
<issue_to_address>
**Retry can duplicate a created task**
When the TES service creates the task but the response is lost or the request times out, `createTask` can commit remotely before a network error reaches the component; the catch path reports failure and retains the form, so retrying posts the same task again. The caller can end up with duplicate tasks despite seeing a failed submission.
Use server-supported idempotency or provide a recovery path for ambiguous outcomes before allowing the same submission to be retried.
</issue_to_address>
### Comment 3
<location path="packages/ecc-client-ga4gh-tes/src/components/runs/runs.ts" line_range="104" />
<code_context>
+ } else if (this.baseUrl) {
+ this._provider = new RestTesProvider(this.baseUrl);
+ } else {
+ this.error =
+ "Please provide either a base URL for the TES API or a custom provider.";
+ }
</code_context>
<issue_to_address>
**Requests keep using the removed provider**
When the component has a configured source and its `provider` and `baseUrl` are later cleared, `initializeProvider()` records the missing-source error but leaves `_provider` set, so reset or reload still calls the old provider and can emit its data.
Clear `_provider` in the no-source branch of `initializeProvider()`.
Also at `packages/ecc-client-ga4gh-tes/src/components/run/run.ts:83-86`.
</issue_to_address>
### Comment 4
<location path="packages/ecc-client-ga4gh-tes/src/components/run/run.ts" line_range="79-86" />
<code_context>
+ }
+
+ private initializeProvider(): void {
+ if (this.provider) {
+ this._provider = this.provider;
+ } else if (this.baseUrl) {
+ this._provider = new RestTesProvider(this.baseUrl);
+ } else {
+ this.error =
+ "Please provide either a base URL for the TES API or a custom provider.";
+ }
+ }
+
</code_context>
<issue_to_address>
**Requests keep using the removed provider**
When the component's configured provider and base URL are later cleared, `initializeProvider()` sets an error but leaves `_provider` set, so `loadTask()` or `loadData()` still calls the old provider and requests continue to the removed service.
Clear `_provider` in the no-source branch of `initializeProvider()`.
Also at `packages/ecc-client-ga4gh-tes/src/components/runs/runs.ts:98-106`.
</issue_to_address>
### Comment 5
<location path="packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts" line_range="243-244" />
<code_context>
+ this.form[key].filter((r) => r.path?.trim() || r.url?.trim());
+ for (const [i, r] of rows("inputs").entries()) {
+ if (!r.path?.trim()) return `Input ${i + 1}: path is required`;
+ if (!r.url?.trim()) return `Input ${i + 1}: url is required`;
+ }
+ for (const [i, r] of rows("outputs").entries()) {
+ if (!r.path?.trim()) return `Output ${i + 1}: path is required`;
</code_context>
<issue_to_address>
**Valid inline inputs are rejected**
When a task input supplies inline `content` without a URL, `validate()` requires every input to have a URL, and the form provides no content field, so a valid TES input is rejected before `createTask` is called.
Expose inline content in the form and accept either a URL or content when validating inputs.
Also at `packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts:636-640`.
</issue_to_address>
### Comment 6
<location path="packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts" line_range="246-247" />
<code_context>
+ if (!r.url?.trim()) return `Input ${i + 1}: url is required`;
+ }
+ for (const [i, r] of rows("outputs").entries()) {
+ if (!r.path?.trim()) return `Output ${i + 1}: path is required`;
+ if (!r.url?.trim()) return `Output ${i + 1}: url is required`;
+ }
+ const badType = [...rows("inputs"), ...rows("outputs")].find(
</code_context>
<issue_to_address>
**Wildcard outputs fail TES validation**
When an output path contains wildcards and its path prefix is empty, `validate()` checks the output path and URL but not its path prefix, so the request reaches the provider without a required prefix and the TES server rejects task creation.
Reject wildcard output paths unless a path prefix is provided.
Also at `packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts:645`.
</issue_to_address>
### Comment 7
<location path="packages/ecc-client-ga4gh-tes/src/providers/rest-tes-provider.ts" line_range="29-30" />
<code_context>
+ * headers (e.g. `Authorization`) or otherwise change requests.
+ */
+ // eslint-disable-next-line class-methods-use-this
+ protected fetch(input: RequestInfo | URL, init?: RequestInit) {
+ return fetch(input, init);
+ }
+
</code_context>
<issue_to_address>
**Hung requests never resolve**
When the TES endpoint or network connection stalls without closing, `fetch()` has no timeout or abort signal, so a TES endpoint that accepts a connection but never responds leaves list, detail, submission, or cancellation operations pending indefinitely; components remain loading or submitting without a result.
Apply a configurable request deadline with an abort signal and surface timeout failures to callers.
</issue_to_address>
### Comment 8
<location path="packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts" line_range="250" />
<code_context>
+ if (!r.url?.trim()) return `Output ${i + 1}: url is required`;
+ }
+ const badType = [...rows("inputs"), ...rows("outputs")].find(
+ (r) => r.type && !["FILE", "DIRECTORY"].includes(r.type.trim())
+ );
+ if (badType) return "Type must be FILE or DIRECTORY";
</code_context>
<issue_to_address>
**Padded file types pass validation**
When a user enters or pastes whitespace around `FILE` or `DIRECTORY`, the type check trims the value only for comparison, while `buildRequest` preserves the original row value. A value such as ` FILE ` therefore passes validation but is sent as an invalid TES enum, and the server rejects the task.
Normalize the type value in the request builder or reject values that are not exactly valid enum strings.
Also at `packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts:210`.
</issue_to_address>
### Comment 9
<location path="packages/ecc-client-ga4gh-tes/src/events/ecc-run-submitted.ts" line_range="1" />
<code_context>
+export type EccRunSubmittedEvent = CustomEvent<Record<string, any>>;
+
+declare global {
</code_context>
<issue_to_address>
**Event details accept arbitrary fields**
When a TypeScript consumer reads an event's detail, the exported event types use `CustomEvent<Record<string, any>>`, so TypeScript accepts misspelled or nonexistent payload fields instead of checking the event-specific contract.
Give each exported event type its specific detail shape.
Also at `packages/ecc-client-ga4gh-tes/src/events/ecc-run-create-failed.ts:1`, `packages/ecc-client-ga4gh-tes/src/events/ecc-run-create-reset.ts:1`, `packages/ecc-client-ga4gh-tes/src/events/ecc-run-log-changed.ts:1`, `packages/ecc-client-ga4gh-tes/src/events/ecc-runs-changed.ts:1`, `packages/ecc-client-ga4gh-tes/src/events/ecc-runs-selected.ts:1`.
</issue_to_address>
### Comment 10
<location path="apps/documentation/pages/docs/tes/providers.mdx" line_range="406" />
<code_context>
+import type { TesProvider } from '@elixir-cloud/tes/providers';
+
+const provider: TesProvider = {
+ getServiceInfo: async () => ({}),
+ listTasks: async () => ({ tasks: [] }),
+ getTask: async (id) => ({ id, state: 'QUEUED' }),
</code_context>
<issue_to_address>
**Provider example fails type-checking**
When a reader copies the documented provider example, `getServiceInfo` returns `{}`, which lacks required `ServiceInfo` fields, so TypeScript rejects the `TesProvider` literal and users cannot copy the example.
Return a `ServiceInfo` object with all required fields from `getServiceInfo`.
</issue_to_address>
### Comment 11
<location path="apps/documentation/pages/docs/tes/components/runs.mdx" line_range="54" />
<code_context>
+| `provider` | `TesProvider` | `undefined` | Custom provider; overrides `baseUrl` (see [Providers](/docs/tes/providers)). |
+| `pageSize` | `number` | `10` | Tasks per page. |
+| `search` | `boolean` | `true` | Show the name-prefix search field. |
+| `filter` | `boolean` | `true` | Show the state filter (applies to the current page). |
+
+## Events
</code_context>
<issue_to_address>
**Filter scope is documented incorrectly**
The table says the state filter applies only to the current page, but the component sends `state` to `listTasks` and resets pagination when it changes; consumers are told to expect different filtering behavior than the implementation provides.
Update the description to state that the filter is sent to the provider and reloads the task list.
</issue_to_address>Linked issues
#443 Cloud Components: migrate `@elixir-cloud/tes` to the v2 provider architecture · 7 of 8 met · 1 to check by hand
| # | Requirement | Status | Notes |
|---|---|---|---|
| 1 | Add the TES provider interface, REST implementation, and events/ | ✅ Met | The provider interface and REST implementation are in src/providers/, and event types are added under src/events/. |
| 2 | Port the runs and create-run components to use the provider | ✅ Met | The listing and task-creation components use a custom provider or construct a RestTesProvider from baseUrl; a task-detail component is also added. |
| 3 | Keep element names stable or document breaking changes | ✅ Met | The create-run element is renamed, and the changeset documents the old and new names. |
| 4 | Align property naming with WES: baseUrl | ✅ Met | The components use baseUrl. |
| 5 | Bump to the 2.0.0-alpha line and update the docs app | ✅ Met | The package version is 2.0.0-alpha.49, and TES documentation pages are added. |
| 6 | Components accept either baseUrl or a custom provider | ✅ Met | Both options are supported, with the custom provider taking precedence when both are set. |
| 7 | Existing tests pass or are ported | ✅ Met | Tests are added for the migrated task-list and task-creation components, including provider and baseUrl paths. |
| 8 | The docs app demo works against a public TES | 👀 Check by hand (runtime) | Run the docs examples against the configured public TES endpoint and confirm the demo can load or submit tasks. |
Sourcery assessment
Needs a human reviewer. 9 findings to address first, and the new form sends task requests to a TES service, where an incorrect request can start workloads, consume compute, or write outputs; reverting the package does not undo tasks that have already run or their effects. The provider migration also promotes @lit/react to a runtime dependency.
Blocking findings: packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts:736, packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts:277, packages/ecc-client-ga4gh-tes/src/components/runs/runs.ts:104, packages/ecc-client-ga4gh-tes/src/components/run/run.ts:86, packages/ecc-client-ga4gh-tes/src/components/run-create/run-create.ts:244, and 4 more
34f1fa1 to
b2263d7
Compare



Summary
Migrate @elixir-cloud/tes package to the v2 architecture
Closes #443
Verification
Declined review findings
Open questions
Checklist
pass.
Commits (at most 50 characters,
imperative mood, no trailing period).
Summary by Sourcery
Migrate the TES package to the v2 providers architecture and deliver the updated task management component suite.
New Features:
TesProviderinterface andRestTesProviderimplementation.Enhancements:
baseUrlconfiguration.Build:
Documentation:
Tests:
Chores: