Skip to content

External Storage: add external_storage_runner to handle core extstore logic#2094

Open
cconstable wants to merge 3 commits into
mainfrom
extstore-core-logic
Open

External Storage: add external_storage_runner to handle core extstore logic#2094
cconstable wants to merge 3 commits into
mainfrom
extstore-core-logic

Conversation

@cconstable

@cconstable cconstable commented May 28, 2026

Copy link
Copy Markdown
Contributor

What was changed

  • Adds a external_storage_runner that handles core external storage logic.
  • Adds some tentative errors and error codes. These just drafts and will change.

Checklist

  • New tests.

@cconstable cconstable requested a review from a team as a code owner May 28, 2026 14:07
@cconstable cconstable changed the title feat(extstore): add external_storage_runner to handle core extstore logic. External Storage: add external_storage_runner to handle core extstore logic May 28, 2026

await t.throwsAsync(() => runExternalStore({ externalStorage, payloads: [makePayload(1)] }), {
instanceOf: ExternalStorageSelectorInvalidDriverError,
message: /TMPRL1109/,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Forgot to update these tests. Updating now.

Comment thread packages/common/src/internal-non-workflow/external-storage-runner.ts Outdated
Comment thread packages/common/src/internal-non-workflow/external-storage-runner.ts Outdated
Comment thread packages/common/src/internal-non-workflow/external-storage-runner.ts Outdated
Comment thread packages/common/src/internal-non-workflow/external-storage-runner.ts Outdated
Comment thread packages/common/src/internal-non-workflow/external-storage-runner.ts Outdated
Comment thread packages/common/src/internal-non-workflow/external-storage-runner.ts Outdated
Comment thread packages/common/src/errors.ts Outdated
Comment thread packages/common/src/errors.ts Outdated
@SymbolBasedInstanceOfError('ExternalStorageNotConfiguredError')
export class ExternalStorageNotConfiguredError extends Error {
constructor(message: string) {
super(`[TMPRL-SDK-00001] ${message}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

According to the rules page this should just emit a log... it doesn't look like an error. Regardless, I'll add the log for it when we actually integrate that piece. I preemptively added it here thinking it would be used in this PR.

@cconstable cconstable force-pushed the extstore-core-logic branch from 96233a3 to 570b9c8 Compare June 12, 2026 15:39
@cconstable cconstable force-pushed the extstore-foundation branch 2 times, most recently from 1b6c2b9 to 447552b Compare June 17, 2026 16:38
Base automatically changed from extstore-foundation to main June 17, 2026 20:09
@cconstable cconstable force-pushed the extstore-core-logic branch from 837ddbe to 5d2a055 Compare June 24, 2026 20:09
Comment thread packages/common/src/errors.ts Outdated
* @experimental
*/
@SymbolBasedInstanceOfError('ExternalStorageDriverOperationFailedError')
export class ExternalStorageDriverOperationFailedError extends Error {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would remove this for now and then apply it across the board to all extstore implementations if we feel the need for it.

Comment thread packages/common/src/errors.ts Outdated
* Thrown when a driver's `store()` or `retrieve()` raises. Wraps the
* underlying error in `cause` (network, auth, rate-limit, S3 5xx, etc).
*
* @experimental

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@internal if we're keeping this. It isn't user-code catchable.

await t.throwsAsync(() => readerRunner.retrieve(storedPayloads), {
instanceOf: ValueError,
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should have more retrieve tests around checking arity, mixed reference and pass through payloads in a single array, sibling driver abort causes all to abort, multiple references to the same driver are batched together, etc.

@cconstable cconstable enabled auto-merge (squash) June 27, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants