-
Notifications
You must be signed in to change notification settings - Fork 100
Add export method feature #2036
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?
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds Meilisearch /export support: new Export types, MeiliSearch.export() method, an integration test for export, a code-sample entry, and CI/docker changes adding a second Meilisearch service bound to host port 7702 for export-related tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester as Test / Caller
participant Client as MeiliSearch Client
participant Server as Meilisearch Server
Tester->>Client: export(options)
Note right of Client #e9f7ef: Build payload (url, apiKey?, payloadSize?, indexes)
Client->>Server: POST /export
Server-->>Client: 202 Accepted + task { uid, type: "export" }
Client-->>Tester: EnqueuedTaskPromise (uid)
par poll status
Tester->>Client: waitForTask(uid) / poll
Client->>Server: GET /tasks/:uid
Server-->>Client: task status (succeeded/failed)
end
Client-->>Tester: Final task result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docker-compose.yml (1)
24-30
: Pin image tag to latest for consistency.Other workflows pin to getmeili/meilisearch:latest. Recommend doing the same here.
Apply this diff:
- export-meilisearch: - image: getmeili/meilisearch + export-meilisearch: + image: getmeili/meilisearch:latesttests/export.test.ts (1)
24-38
: Parameterize hardcoded MeiliSearch URLs for portability.Avoid embedding
http://127.0.0.1:7701
in tests—read both the export host and BAD_HOST from env vars with sensible defaults:diff --git a/tests/export.test.ts b/tests/export.test.ts --- a/tests/export.test.ts +++ b/tests/export.test.ts @@ - test(`${ms.export.name} method`, async () => { - const task = await ms - .export({ - url: "http://127.0.0.1:7701", +const EXPORT_URL = process.env.MEILISEARCH_EXPORT_URL ?? "http://127.0.0.1:7701"; + +test(`${ms.export.name} method`, async () => { + const task = await ms + .export({ + url: EXPORT_URL, apiKey: "masterKey", payloadSize: "50MiB", indexes: { [INDEX_UID]: { filter: "beep = boop", overrideSettings: true }, }, }) .waitTask({ timeout: 60_000 });diff --git a/tests/utils/meilisearch-test-utils.ts b/tests/utils/meilisearch-test-utils.ts --- a/tests/utils/meilisearch-test-utils.ts +++ b/tests/utils/meilisearch-test-utils.ts @@ -const BAD_HOST = "http://127.0.0.1:7701"; +const BAD_HOST = process.env.MEILISEARCH_BAD_HOST_URL ?? "http://127.0.0.1:7701";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.code-samples.meilisearch.yaml
(1 hunks).github/workflows/meilisearch-prototype-tests.yml
(1 hunks).github/workflows/pre-release-tests.yml
(1 hunks).github/workflows/tests.yml
(1 hunks)docker-compose.yml
(1 hunks)src/meilisearch.ts
(2 hunks)src/types/export.ts
(1 hunks)src/types/index.ts
(1 hunks)src/types/task_and_batch.ts
(1 hunks)tests/export.test.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
PR: meilisearch/meilisearch-js#1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and the main CI workflow (.github/workflows/tests.yml) use `getmeili/meilisearch:latest` to maintain consistency between local development and CI testing environments.
Applied to files:
docker-compose.yml
.github/workflows/pre-release-tests.yml
.github/workflows/tests.yml
.github/workflows/meilisearch-prototype-tests.yml
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
PR: meilisearch/meilisearch-js#1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and CI workflows use the latest Meilisearch image (getmeili/meilisearch:latest) for consistency between development and testing environments.
Applied to files:
docker-compose.yml
.github/workflows/pre-release-tests.yml
.github/workflows/tests.yml
.github/workflows/meilisearch-prototype-tests.yml
🧬 Code graph analysis (2)
src/types/export.ts (1)
src/types/types.ts (1)
Filter
(170-170)
src/meilisearch.ts (2)
src/types/export.ts (1)
ExportOptions
(21-30)src/types/task_and_batch.ts (1)
EnqueuedTaskPromise
(160-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-tests (Node.js 20)
- GitHub Check: integration-tests (Node.js 22)
🔇 Additional comments (8)
src/types/task_and_batch.ts (1)
55-57
: TaskType includes Export — LGTM.Matches test expectation task.type === "export" and server naming.
src/types/index.ts (1)
2-2
: Re-export of export types — LGTM.Public surface now exposes ExportOptions and related types.
src/meilisearch.ts (2)
37-38
: Type import — LGTM.Importing ExportOptions from types index is consistent with the rest.
551-558
: New export() method — LGTM.Consistent with dumps/snapshots (POST + enqueued task). Endpoint path name aligns with tests.
.github/workflows/meilisearch-prototype-tests.yml (1)
47-53
: Second Meilisearch service — LGTM.Provides dedicated export target on 7701 for integration tests.
.github/workflows/tests.yml (1)
36-42
: Second Meilisearch service — LGTM.Parity with docker-compose and other workflows; enables export tests reliably.
src/types/export.ts (1)
1-30
: Type definitions align with the export API contractFilter and override settings mirror the server-side struct, and the options shape matches the documented payload. Looks good.
.github/workflows/pre-release-tests.yml (1)
37-50
: Second Meilisearch service configuration looks consistentImage, env, and port mapping mirror the primary service while exposing 7701 for export tests, keeping parity with the existing workflow setup.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker-compose.yml (1)
24-30
: Explicitly pin the export image tag.
Lines 24-30 currently pullgetmeili/meilisearch
without an explicit tag. To keep parity with our CI workflows and avoid tracking a moving default, please pin both services togetmeili/meilisearch:latest
.meilisearch: - image: getmeili/meilisearch + image: getmeili/meilisearch:latest @@ export-meilisearch: - image: getmeili/meilisearch + image: getmeili/meilisearch:latestBased on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/meilisearch-prototype-tests.yml
(1 hunks).github/workflows/pre-release-tests.yml
(1 hunks).github/workflows/tests.yml
(1 hunks)docker-compose.yml
(1 hunks)tests/export.test.ts
(1 hunks)tests/utils/meilisearch-test-utils.ts
(1 hunks)tests/utils/tasks-and-batches.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/pre-release-tests.yml
- .github/workflows/meilisearch-prototype-tests.yml
- tests/export.test.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
PR: meilisearch/meilisearch-js#1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and the main CI workflow (.github/workflows/tests.yml) use `getmeili/meilisearch:latest` to maintain consistency between local development and CI testing environments.
Applied to files:
.github/workflows/tests.yml
docker-compose.yml
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
PR: meilisearch/meilisearch-js#1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and CI workflows use the latest Meilisearch image (getmeili/meilisearch:latest) for consistency between development and testing environments.
Applied to files:
.github/workflows/tests.yml
docker-compose.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-tests (Node.js 22)
- GitHub Check: integration-tests (Node.js 20)
🔇 Additional comments (3)
.github/workflows/tests.yml (1)
36-42
: Nice parity for the second Meilisearch service.
Mirroring the primary service config on port 7702 keeps the integration environment consistent for the new export coverage.tests/utils/meilisearch-test-utils.ts (1)
200-204
: Task assertion updated for export tasks.
Withexport
included here, the shared assertions stay aligned with the expandedTaskType
union.tests/utils/tasks-and-batches.ts (1)
30-34
: Helper keeps pace with the new task type.
Addingexport
ensures the task-type assertions accept the export jobs emitted by the API.
Seems like this export method doesn't quite work. It shouldn't take 1 minute + to transfer an index with one tiny document and its default settings. |
TODO: Task detail types:
|
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.
Hi @flevi29 and thanks for this PR 🙌 Just left a small suggestion to align the code sample with the meilisearch docs.
Co-authored-by: Laurent Cazanove <[email protected]>
Hi @Strift, someone needs to check this out. I think it's a bug. |
tests/export.test.ts
Outdated
test(`${ms.export.name} method`, async () => { | ||
const task = await ms | ||
.export({ | ||
url: "http://127.0.0.1:7702", |
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.
Would this work in Docker? Shouldn't we use the service's internal name, e.g., http://export-meilisearch:7700
?
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.
Oh yeah, you're right. Nevertheless it should err and not hang, so it still counts as a bug.
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.
Okay, now I get:
{
"code": "internal",
"link": "https://docs.meilisearch.com/errors#internal",
"message": "Failed to export documents to remote server missing_payload (invalid_request): A ndjson payload is missing. <https://docs.meilisearch.com/errors#missing_payload>",
"type": "internal",
}
Not sure what the problem is here.
Pull Request
Related issue
Fixes #1989
What does this PR do?
Summary by CodeRabbit
New Features
Documentation
Tests
Chores