Fix outdated Pipfile.lock and pipenv install inconsistencies across CI jobs#1223
Fix outdated Pipfile.lock and pipenv install inconsistencies across CI jobs#1223Strift merged 1 commit intomeilisearch:mainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request updates GitHub Actions workflows to enforce strict dependency resolution using pipenv's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
meilisearch/_httprequests.py (1)
204-213: LGTM! Black formatting applied correctly.The reformatting is consistent with the changes in the
send_requestmethod above.Optional refactor: Extract duplicated exception handling.
The exception handling for
InvalidSchema(lines 204-213) is duplicated fromsend_request(lines 84-93). Consider extracting this into a helper method to reduce duplication.♻️ Optional refactor to eliminate duplication
You could extract the common exception handling into a helper method:
def _handle_invalid_schema(self, err: requests.exceptions.InvalidSchema) -> None: """Handle InvalidSchema exception with helpful URL error message.""" if "://" not in self.config.url: raise MeilisearchCommunicationError( f""" Invalid URL {self.config.url}, no scheme/protocol supplied. Did you mean https://{self.config.url}? """ ) from err raise MeilisearchCommunicationError(str(err)) from errThen replace both exception handlers with a call to this method:
except requests.exceptions.InvalidSchema as err: self._handle_invalid_schema(err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@meilisearch/_httprequests.py` around lines 204 - 213, Extract the duplicated InvalidSchema handling into a helper method (e.g., _handle_invalid_schema(self, err: requests.exceptions.InvalidSchema)) and move the logic that checks for "://" in self.config.url and raises MeilisearchCommunicationError (preserving the "from err") into that method; then replace both except requests.exceptions.InvalidSchema blocks (the one in send_request and the one in the shown method) with a single call to self._handle_invalid_schema(err) to remove duplication while keeping identical behavior and stack traces..github/workflows/tests.yml (1)
50-50: Deduplicate the repeated3.9value across jobs.These commands repeat the same minimum Python value. Centralizing it in a workflow-level env var will reduce drift when the minimum version changes.
♻️ Proposed refactor
name: Tests +env: + MIN_PYTHON_VERSION: "3.9" on: pull_request: @@ - - name: Install dependencies - run: pipenv install --dev --python=3.9 --deploy + - name: Install dependencies + run: pipenv install --dev --python=${{ env.MIN_PYTHON_VERSION }} --deploy @@ - - name: Install dependencies - run: pipenv install --dev --python=3.9 --deploy + - name: Install dependencies + run: pipenv install --dev --python=${{ env.MIN_PYTHON_VERSION }} --deploy @@ - - name: Install dependencies - run: pipenv install --dev --python=3.9 --deploy + - name: Install dependencies + run: pipenv install --dev --python=${{ env.MIN_PYTHON_VERSION }} --deploy @@ - - name: Install dependencies - run: pipenv install --dev --python=3.9 --deploy + - name: Install dependencies + run: pipenv install --dev --python=${{ env.MIN_PYTHON_VERSION }} --deployAlso applies to: 67-67, 84-84, 101-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/tests.yml at line 50, Introduce a workflow-level environment variable (e.g., MIN_PYTHON or PYTHON_MIN) and replace all hardcoded "3.9" occurrences in the jobs' pipenv install commands with that variable (the run lines containing "pipenv install --dev --python=3.9 --deploy" across the workflow, including the occurrences noted at the other jobs). Update the top-level workflow "env:" block to define the variable (e.g., PYTHON_MIN=3.9) so all jobs reference the single source of truth instead of repeating the literal version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/tests.yml:
- Line 50: Introduce a workflow-level environment variable (e.g., MIN_PYTHON or
PYTHON_MIN) and replace all hardcoded "3.9" occurrences in the jobs' pipenv
install commands with that variable (the run lines containing "pipenv install
--dev --python=3.9 --deploy" across the workflow, including the occurrences
noted at the other jobs). Update the top-level workflow "env:" block to define
the variable (e.g., PYTHON_MIN=3.9) so all jobs reference the single source of
truth instead of repeating the literal version.
In `@meilisearch/_httprequests.py`:
- Around line 204-213: Extract the duplicated InvalidSchema handling into a
helper method (e.g., _handle_invalid_schema(self, err:
requests.exceptions.InvalidSchema)) and move the logic that checks for "://" in
self.config.url and raises MeilisearchCommunicationError (preserving the "from
err") into that method; then replace both except
requests.exceptions.InvalidSchema blocks (the one in send_request and the one in
the shown method) with a single call to self._handle_invalid_schema(err) to
remove duplication while keeping identical behavior and stack traces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01fee541-9e7a-4d8e-9398-39c42b060f1a
⛔ Files ignored due to path filters (1)
Pipfile.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/documentation.yml.github/workflows/pre-release-tests.yml.github/workflows/tests.ymlmeilisearch/_httprequests.py
|
|
Strift
left a comment
There was a problem hiding this comment.
Thanks for this!
I don't have the bandwidth to migrate to UV, but would be happy to help by reviewing the PR :)
Will do. 🎯 |
Pull Request
Related issue
Fixes #1220
What does this PR do?
% pipenv lock--> Updated Pipfile.lock (which implicitly updated every time CI jobs ran)--deployflag, which is recommended withpipenvin CI pipelines. This will fail early ifPipfilegoes out of sync withPipfile.lock. See${{ matrix.python-version }}reference, which just resolved to empty, and instead added the minimum supported Python version for the project, which isPython 3.9black --checkcommand.github/workflows/documentation.ymlmade this job use Python 3.9 because all other jobs uses this (3.9) minimum required version for this projectExtra
1 - Working clean environment:

PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit