-
Notifications
You must be signed in to change notification settings - Fork 0
Upgrade Harper to v5 (harperdb → harper) #3
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
base: main
Are you sure you want to change the base?
Changes from all commits
088233c
278748a
427bb3e
f9ee471
bd229af
083e692
9931edc
015d47b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| name: Integration Tests | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| pull_request: | ||
| workflow_dispatch: | ||
| inputs: | ||
| node-version: | ||
| description: 'Node.js version' | ||
| required: true | ||
| type: choice | ||
| default: 'all' | ||
| options: | ||
| - 'all' | ||
| - '22' | ||
| - '24' | ||
| - '26' | ||
|
|
||
| jobs: | ||
| generate-node-version-matrix: | ||
| name: Generate Node Version Matrix | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| node-versions: ${{ steps.set-node-versions.outputs.node-versions }} | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | ||
|
|
||
| - name: Set Node versions | ||
| id: set-node-versions | ||
| env: | ||
| NODE_VER: ${{ github.event.inputs.node-version }} | ||
| run: | | ||
| if [ "$NODE_VER" == "all" ] || [ -z "$NODE_VER" ]; then | ||
| echo "node-versions=[22, 24, 26]" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "node-versions=[$NODE_VER]" >> $GITHUB_OUTPUT | ||
| fi | ||
|
|
||
| integration-tests: | ||
| name: Integration Tests (Node ${{ matrix.node-version }}) | ||
| needs: [generate-node-version-matrix] | ||
| runs-on: ubuntu-latest | ||
|
|
||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| node-version: ${{ fromJSON(needs.generate-node-version-matrix.outputs.node-versions) }} | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Build TypeScript | ||
| run: npm run build | ||
|
|
||
| - name: Run integration tests | ||
| run: npm run test:integration | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old The removed Consider adding a step before this one to also run |
||
| env: | ||
| HARPER_INTEGRATION_TEST_LOG_DIR: /tmp/harper-test-logs | ||
| FORCE_COLOR: '1' | ||
|
|
||
| - name: Upload Harper logs on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | ||
| with: | ||
| name: harper-logs-node-${{ matrix.node-version }} | ||
| path: /tmp/harper-test-logs/ | ||
| retention-days: 7 | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| declare module 'harperdb'; | ||
| declare module 'harper'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import sharp from 'sharp'; | ||
| import { Resource, databases, logger } from 'harperdb'; | ||
| import { Resource, databases, logger, createBlob } from 'harper'; | ||
| import { parseCacheKey, formatToContentType } from './utils/index.js'; | ||
| import type { User } from './types/index.js'; | ||
| import { randomUUID } from 'crypto'; | ||
|
|
@@ -302,7 +302,7 @@ export class Images extends Resource { | |
| for (const variant of variants || []) { | ||
| const vId = (variant as any)?.id ?? variant; | ||
| if (typeof vId === 'string') { | ||
| await VariantsTable.delete(vId); | ||
| await VariantsTable.invalidate(vId); | ||
| } | ||
| } | ||
|
Comment on lines
302
to
307
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The loop performs sequential asynchronous const invalidations = (variants || []).map(async (variant) => {
const vId = (variant as any)?.id ?? variant;
if (typeof vId === 'string') {
await VariantsTable.invalidate(vId);
}
});
await Promise.all(invalidations); |
||
| } catch (err: any) { | ||
|
|
@@ -317,4 +317,5 @@ export class Images extends Resource { | |
| } | ||
| } | ||
|
|
||
| VariantsTable.sourcedFrom(ImagesTable); | ||
| // VariantsTable variant generation is handled manually in ImageVariant.get(); | ||
| // sourcedFrom is not used here because variant IDs differ from image IDs. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding
cache: npmto thesetup-nodestep for faster CI runsThe removed
test.yamlworkflow usedcache: npmwithsetup-node. The new workflow omits it, so every matrix run does a coldnpm ci(cold install of the fullharper+ nativesharpbinaries). Addingcache: npmto this step:...will cache the npm download cache between runs, reducing install time significantly (especially for
sharpwith its optional native binaries). Low-effort win.