fix: specifying a target version for a source does not actually install that version #651
Conversation
📝 WalkthroughWalkthroughThis changeset updates the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Executor as get_connector_executor
Note over Executor: Calculate install_method_count
Caller->>Executor: Call get_connector_executor(version, pip_url, ...)
alt Both version and pip_url provided
Executor-->>Caller: Raise PyAirbyteInputError ("Both version and pip_url provided. Specify version in pip_url.")
else Valid parameters provided
Executor->>Executor: Process installation method
alt InstallType.PYTHON and version provided
Executor->>Executor: Append version to pip_url
end
Executor-->>Caller: Continue execution
end
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte/_executors/util.py(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
airbyte/_executors/util.py (1)
airbyte/exceptions.py (1)
PyAirbyteInputError(201-210)
🪛 Ruff (0.8.2)
airbyte/_executors/util.py
154-154: Line too long (131 > 100)
(E501)
160-160: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🪛 GitHub Actions: Run Linters
airbyte/_executors/util.py
[error] 1-1: Ruff formatting check failed. 1 file would be reformatted. Please run 'ruff format' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (No Creds)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (1)
airbyte/_executors/util.py (1)
211-211: Perfect fix for the version specification issue!This line change is the heart of the fix - it ensures that when a user specifies a version, it's properly appended to the pip_url with the
==syntax that pip requires for exact version installations. This directly addresses the issue where specifying a target version wasn't actually installing that version.Before this change, the system would likely ignore the
versionparameter when constructing the pip command, which explains why it was installing a different version than requested.
marcosmarxm
changed the title
|
/fix-pr
|
Fixes #622
Example:
PyAirbyte logs:
With this change we make sure the
pip_urluses the targetversion.Also fix a possible bug in case users specify
pip_urlandversionwhere it will install the static value ofpip_urlNow it will throws an error alerting users to use pip_url OR version.
My initial idea was to change the following code:
PyAirbyte/airbyte/_executors/python.py
Lines 56 to 60 in ee5cb2d
But you'll always receive the
pip_urland the executor doesn't have knowledge if it was the user or the (current code block) created the pip_url variable.Summary by CodeRabbit
versionandpip_urlfrom being specified simultaneously.