Skip to content

Document the optional AWS SDK peer requirement for S3 import/export jobs (v5.3) - #672

Open
heskew wants to merge 3 commits into
mainfrom
docs/s3-optional-peer
Open

heskew wants to merge 3 commits into
mainfrom
docs/s3-optional-peer

Conversation

@heskew

@heskew heskew commented Sep 15, 2026

Copy link
Copy Markdown
Member

Docs companion to HarperFast/harper#2609: as of Harper v5.3 the AWS S3 SDK is an optional peer dependency, so the import_from_s3 and export_to_s3 job operations require @aws-sdk/client-s3 + @aws-sdk/lib-storage to be installed where Harper can resolve them, and fail with an actionable 501 when absent. This adds a v5.3 version badge to both sections of reference/database/jobs.md with the two explicit install procedures (from the instance root, or --global when Harper is installed globally).

For the human reviewer

Gated on harper#2609's rollout decision: merge only with/after that PR, and the badge assumes it ships in v5.3.0 — if the milestone moves, the badge version moves with it.

Verification

Two cross-model rounds on this branch (codex graded leg): round 1 flagged the install command as ambiguous between the two locations (fixed — both procedures now explicit); round 2 on this exact HEAD is LGTM with zero findings. The reviewer verified the #import-from-s3 anchor resolves uniquely and git diff --check passes. Prettier could not run locally (the shared config package isn't installed in this worktree); the repo's format CI is the gate.

Complexity: easy

🤖 Generated with Claude Code

https://claude.ai/code/session_012GGq9WbGH3m8grGuKb8rYe

Review-Coverage: authored=claude; ran=codex; declined=gemini,cursor-grok,cursor-composer,domain; rounds=2; full=1 @ 2a200f2

Human-Review-Need: 2 @ 2a200f2

heskew and others added 2 commits September 15, 2026 08:52
…xport_to_s3 (v5.3)

As of Harper v5.3 the AWS S3 SDK is an optional peer dependency (HarperFast/harper#2609, #2607): the S3 import/export operations require @aws-sdk/client-s3 and @aws-sdk/lib-storage to be installed in the Harper instance root or globally alongside Harper, and fail with an actionable 501 when absent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GGq9WbGH3m8grGuKb8rYe

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request updates the database jobs documentation to specify that the AWS SDK is an optional peer dependency for S3 import and export operations as of version 5.3.0. The feedback suggests wrapping these critical requirements in caution blocks to improve visibility and formatting the installation commands for better readability.

Comment thread reference/database/jobs.md Outdated

Imports CSV or JSON files from an AWS S3 bucket.

<VersionBadge type="changed" version="v5.3.0" /> — the AWS SDK backing this operation is an optional peer dependency. Install it where Harper can resolve it: run `npm install @aws-sdk/client-s3 @aws-sdk/lib-storage` from the Harper instance root (`rootPath`), or — when Harper itself is installed globally — `npm install --global @aws-sdk/client-s3 @aws-sdk/lib-storage`. Without it, the operation fails with a `501` error naming this command.

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.

medium

This note describes a critical requirement for the operation to function. To ensure users don't miss this, it's best to place it inside a :::caution block for better visibility, as per our general documentation guidelines for critical caveats.

I've also restructured the instructions to be more readable by separating the local and global installation steps. This should make it easier for users to follow.

Suggested change
<VersionBadge type="changed" version="v5.3.0" /> — the AWS SDK backing this operation is an optional peer dependency. Install it where Harper can resolve it: run `npm install @aws-sdk/client-s3 @aws-sdk/lib-storage` from the Harper instance root (`rootPath`), or — when Harper itself is installed globally — `npm install --global @aws-sdk/client-s3 @aws-sdk/lib-storage`. Without it, the operation fails with a `501` error naming this command.
:::caution Peer Dependencies Required
<VersionBadge type="changed" version="v5.3.0" />
As of v5.3.0, the AWS SDK is an optional peer dependency for S3 operations. To use `import_from_s3`, you must install the required packages where Harper can resolve them.
- **For a local Harper installation**, run this command from your Harper instance root (`rootPath`):
```bash
npm install @aws-sdk/client-s3 @aws-sdk/lib-storage
  • For a global Harper installation, run:
    npm install --global @aws-sdk/client-s3 @aws-sdk/lib-storage

Without these dependencies, the operation will fail with a 501 error.
:::


<details>
<summary>References</summary>

1. When documenting critical caveats, potential runtime errors, or limitations, format them using a `:::caution` block (or appropriate admonition) to ensure they are prominent and visible, while maintaining consistency with the document's existing formatting patterns.
</details>


<VersionBadge type="changed" version="v4.3.0" /> — `search_by_conditions` added as a supported search operation

<VersionBadge type="changed" version="v5.3.0" /> — the AWS SDK backing this operation is an optional peer dependency; see the installation note under [Import from S3](#import-from-s3). Without it, the operation fails with a `501` 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.

medium

For consistency with the import_from_s3 section and to maintain visibility for this critical requirement, this note should also be placed within a :::caution block.

Suggested change
<VersionBadge type="changed" version="v5.3.0" /> — the AWS SDK backing this operation is an optional peer dependency; see the installation note under [Import from S3](#import-from-s3). Without it, the operation fails with a `501` error.
:::caution Peer Dependencies Required
<VersionBadge type="changed" version="v5.3.0" />
This operation requires the same AWS SDK peer dependencies as `import_from_s3`. See the installation note under [Import from S3](#import-from-s3). Without the dependencies, the operation will fail with a `501` error.
:::
References
  1. When documenting critical caveats, potential runtime errors, or limitations, format them using a :::caution block (or appropriate admonition) to ensure they are prominent and visible, while maintaining consistency with the document's existing formatting patterns.

@github-actions
github-actions Bot temporarily deployed to pr-672 September 15, 2026 16:42 Inactive
@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-672

This preview will update automatically when you push new commits.

Follows harper#2609's rollout shape: image users keep S3 import/export out of the box; the install note applies to npm installs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GGq9WbGH3m8grGuKb8rYe
@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-672

This preview will update automatically when you push new commits.

@heskew
heskew marked this pull request as ready for review September 19, 2026 01:00
@heskew
heskew requested a review from a team as a code owner September 19, 2026 01:00

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The new dependency guidance clearly distinguishes Docker and npm installations. No confirmed blocking issues remain on the changed lines.


Reviewed e5ec671

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