Add meilisearch target version and logic#2035
Open
flevi29 wants to merge 22 commits intomeilisearch:mainfrom
Open
Add meilisearch target version and logic#2035flevi29 wants to merge 22 commits intomeilisearch:mainfrom
meilisearch target version and logic#2035flevi29 wants to merge 22 commits intomeilisearch:mainfrom
Conversation
📝 WalkthroughWalkthroughReplaces GitHub Actions Meilisearch service containers with docker-compose + Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Docker as Docker Compose
participant Meili as Meilisearch
participant Vitest as Vitest GlobalSetup
participant Tests as Test Runner
CI->>Docker: run `docker compose --env-file .conf up -d`
Docker-->>Meili: start Meilisearch (v${TARGET_VERSION}, ${PORT})
Vitest->>Vitest: load `.conf` (MASTER_KEY, PORT)
Vitest->>Meili: poll `client.health()` every 250ms (Abort after 6s)
alt Meilisearch becomes healthy
Meili-->>Vitest: health OK
Vitest->>Tests: provide PORT/MASTER_KEY → Tests start
else timeout / error
Meili-->>Vitest: no response / error
Vitest-->>Tests: throw connection unsuccessful error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2035 +/- ##
=======================================
Coverage 97.95% 97.95%
=======================================
Files 15 15
Lines 635 635
Branches 104 105 +1
=======================================
Hits 622 622
Misses 12 12
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…arch-js into lock-meiliserach-version
4 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pre-release-tests.yml (1)
19-35:⚠️ Potential issue | 🔴 CriticalCritical: Missing
meilisearch-versionjob andTARGET_VERSIONnot passed to Docker Compose.Two issues will cause this workflow to fail:
Line 23 declares
needs: ["meilisearch-version"], but themeilisearch-versionjob is not defined in this workflow file. This will cause the workflow to fail immediately.Line 35 runs
docker compose up -dwithout settingTARGET_VERSION. Thedocker-compose.ymlexpects this variable to construct the image tag (getmeili/meilisearch-enterprise:v${TARGET_VERSION}).Compare with
.github/workflows/meilisearch-prototype-tests.ymlwhich correctly defines themeilisearch-versionjob and passes the version:run: TARGET_VERSION=${{ needs.meilisearch-version.outputs.version }} docker compose up -d🐛 Proposed fix
Either:
- Add the
meilisearch-versionjob (similar tomeilisearch-prototype-tests.yml) and passTARGET_VERSIONto docker compose, OR- If pre-release tests should use a fixed version from
.env.example, remove theneeds: ["meilisearch-version"]line.If using a dynamic version:
+ meilisearch-version: + runs-on: ubuntu-latest + outputs: + version: ${{ steps.grep-step.outputs.meilisearch_version }} + steps: + - uses: actions/checkout@v6 + - name: Get pre-release Meilisearch version + id: grep-step + run: | + # Add logic to determine the pre-release version + echo "meilisearch_version=<VERSION>" >> $GITHUB_OUTPUT + integration_tests: runs-on: ubuntu-latest if: github.event_name != 'pull_request' || startsWith(github.base_ref, 'bump-meilisearch-v') || startsWith(github.base_ref, 'pre-release-beta') needs: ["meilisearch-version"] ... - name: Set up Meilisearch - run: docker compose up -d + run: TARGET_VERSION=${{ needs.meilisearch-version.outputs.version }} docker compose up -d🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pre-release-tests.yml around lines 19 - 35, The workflow's integration_tests job references a missing job name ("meilisearch-version") and fails to pass TARGET_VERSION into the Docker Compose step; update the workflow by either (A) adding a meilisearch-version job that outputs the chosen version and keep needs: ["meilisearch-version"], then change the "Set up Meilisearch" step (the docker compose up -d run) to run as: TARGET_VERSION=${{ needs.meilisearch-version.outputs.version }} docker compose up -d, or (B) remove needs: ["meilisearch-version"] and explicitly set TARGET_VERSION from a fixed source (e.g., .env.example) before running docker compose; reference the integration_tests job, the needs declaration, and the "Set up Meilisearch" docker compose step when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/tests.yml:
- Around line 39-40: The CI currently runs "docker compose up -d" in the "Set up
Meilisearch" step but doesn't wait for the Meilisearch HTTP server to be ready,
causing race conditions; add an explicit readiness check immediately after that
step that polls Meilisearch's health endpoint (e.g., GET
http://localhost:7700/health) with a timeout/retry loop and exits non‑zero if
unchanged, or alternatively add a Docker healthcheck to the docker-compose
service and replace "docker compose up -d" with "docker compose up -d --wait" so
the next step (the test step) only runs once Meilisearch is healthy.
In `@CONTRIBUTING.md`:
- Around line 62-68: The CONTRIBUTING.md snippet doesn't explain how to select a
non-default Meilisearch version; update the doc block that currently describes
copying .env and running docker compose up to explicitly instruct contributors
to edit the TARGET_VERSION variable in the .env file to the desired Meilisearch
release before running docker compose up -d, and mention the .env/TARGET_VERSION
pairing so it's discoverable when targeting a non-default version.
In `@playgrounds/javascript/vite.config.ts`:
- Around line 4-5: After calling loadEnvFile("../../.env"), validate that
env.PORT and env.MASTER_KEY are defined (the symbols to check are env, PORT,
MASTER_KEY as used in the destructuring) and fail fast with a clear error if
either is missing; update the code around loadEnvFile and the destructuring so
you check (or assert) env.PORT and env.MASTER_KEY and throw or log a descriptive
error and exit (or throw) instead of allowing undefined values to be passed into
the Vite define replacements for __PORT__ and __MASTER_KEY__.
In `@tests/multi_modal_search.test.ts`:
- Around line 3-5: Replace the deprecated JSON import assertion by updating the
movies import in tests/multi_modal_search.test.ts: remove the old assert syntax
and use the Node.js 20+ import attributes form so the module is imported as a
JSON module (i.e., import the JSON as the movies symbol using the modern
import-attributes/assertion style compatible with the rest of the codebase and
runtime).
In `@tests/setup/index.ts`:
- Around line 5-13: After calling loadEnvFile(), validate that env.PORT and
env.MASTER_KEY are defined and non-empty before constructing host or new
Meilisearch; if either is missing throw or log a clear error (e.g., "Missing
required env var PORT/MASTER_KEY") so the failure is explicit. Locate the
validation near loadEnvFile() and perform the check on env.PORT and
env.MASTER_KEY prior to using them to build host or instantiate client so client
(the Meilisearch instance) is only created when configuration is valid.
---
Outside diff comments:
In @.github/workflows/pre-release-tests.yml:
- Around line 19-35: The workflow's integration_tests job references a missing
job name ("meilisearch-version") and fails to pass TARGET_VERSION into the
Docker Compose step; update the workflow by either (A) adding a
meilisearch-version job that outputs the chosen version and keep needs:
["meilisearch-version"], then change the "Set up Meilisearch" step (the docker
compose up -d run) to run as: TARGET_VERSION=${{
needs.meilisearch-version.outputs.version }} docker compose up -d, or (B) remove
needs: ["meilisearch-version"] and explicitly set TARGET_VERSION from a fixed
source (e.g., .env.example) before running docker compose; reference the
integration_tests job, the needs declaration, and the "Set up Meilisearch"
docker compose step when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c89aa74-7397-4bd2-bbb7-18f6f4a106a0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (56)
.env.example.github/actions/set-up-node/action.yml.github/workflows/meilisearch-prototype-tests.yml.github/workflows/pre-release-tests.yml.github/workflows/tests.ymlCONTRIBUTING.mddocker-compose.ymlpackage.jsonplaygrounds/javascript/package.jsonplaygrounds/javascript/src/meilisearch.tsplaygrounds/javascript/src/vite-env.d.tsplaygrounds/javascript/vite.config.tssrc/meilisearch.tstests/chat-settings.test.tstests/chat-workspaces.test.tstests/client.test.tstests/displayed_attributes.test.tstests/distinct_attribute.test.tstests/documents.test.tstests/dump.test.tstests/embedders.test.tstests/errors.test.tstests/experimental-features.test.tstests/facet_search_settings.test.tstests/faceting.test.tstests/fields.test.tstests/filterable_attributes.test.tstests/get_search.test.tstests/http-requests.test.tstests/index.test.tstests/keys.test.tstests/localized_attributes.test.tstests/multi_modal_search.test.tstests/pagination.test.tstests/prefix_search_settings.test.tstests/ranking_rules.test.tstests/raw_document.test.tstests/search.test.tstests/search_cutoff_ms.test.tstests/searchable_attributes.test.tstests/settings.test.tstests/setup/index.tstests/snapshots.test.tstests/sortable_attributes.test.tstests/stop_words.test.tstests/synonyms.test.tstests/task.test.tstests/tasks-and-batches.test.tstests/token.test.tstests/typed_search.test.tstests/typo_tolerance.test.tstests/utils/assertions/error.tstests/utils/assertions/tasks-and-batches.tstests/utils/meilisearch-test-utils.tstests/webhooks.test.tsvite.config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pre-release-tests.yml:
- Around line 33-35: The docker compose step currently runs `docker compose up
-d` without passing the resolved pre-release version, so it may start the wrong
Meilisearch image; update the workflow to inject the resolved version into the
compose environment (e.g., set an env entry like TARGET_VERSION: ${{
needs.resolve_version.outputs.target_version }} on the job or step, or restore
the version-resolving job and consume its output) and ensure the `docker compose
up -d` step reads TARGET_VERSION (or uses an overridden .env) so compose starts
the correct Meilisearch image; reference the `docker compose up -d` step and the
version-resolving job/output when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c4afc3c-b861-41a2-a3ae-4bb34362f5c9
📒 Files selected for processing (1)
.github/workflows/pre-release-tests.yml
Collaborator
Author
|
@Strift I decided to give it one last shot with a more code agnostic solution 🤞. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
What does this PR do?
/.conf, readable both by Docker and Node.js, and adapted all code to use this configuration filedocker-compose.ymlinstead of lengthy repetitive service descriptionsdocekr-compose.yml"package" entrySummary by CodeRabbit
New Features
Chores