Skip to content

Fix: dependencies were pinned too strictly#165

Open
hoh wants to merge 1 commit intomainfrom
hoh-relax-dependencies
Open

Fix: dependencies were pinned too strictly#165
hoh wants to merge 1 commit intomainfrom
hoh-relax-dependencies

Conversation

@hoh
Copy link
Member

@hoh hoh commented Sep 14, 2024

This prevented installing the SDK on some systems
such as Nix 24.05.

This prevented installing the SDK on some systems
such as Nix 24.05.
@github-actions github-actions bot added the BLUE This PR is simple and straightforward. label Sep 14, 2024
@github-actions
Copy link

"summary": "This PR involves a minor update to the dependencies list in pyproject.toml. The changes from specific versions (4.3.1 and 6.3.0) to more general versions (eth_typing>=4.0.0 and web3>=6.3.0) are likely to be non-breaking and do not introduce significant risks. The update is focused on flexibility and compatibility, which aligns with the 'BLUE' category, indicating a low complexity PR.",
"highlight": [
"- "eth_typing==4.3.1",",
"+ "eth_typing>=4.0.0",",
"- "web3==6.3.0",",
"+ "web3>=6.3.0","
]
}

@hoh hoh requested a review from philogicae September 16, 2024 12:21
"aioresponses>=0.7.6",
"aleph-superfluid>=0.2.1",
"eth_typing==4.3.1",
"web3==6.3.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to add an upper bound version to enforce semver to avoid randomly breaking our installation.

Copy link

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change relaxes two overly strict pinned dependencies (eth_typing==4.3.1 and web3==6.3.0) to minimum version bounds (>=4.0.0 and >=6.3.0 respectively), which is the correct approach for a library. Pinning exact versions in a library's dependencies causes conflicts for consumers who need different versions of those packages, and was breaking installation on systems like Nix 24.05. The lower bounds chosen are reasonable — web3>=6.3.0 keeps the minimum at the previously pinned version, ensuring no regression in required functionality. The only mild concern is that eth_typing>=4.0.0 allows versions below the previously pinned 4.3.1, which could theoretically expose compatibility issues if features introduced between 4.0.0 and 4.3.1 are used; however, this is a low risk and can be tightened later if issues arise.

pyproject.toml (line 35): Minor: eth_typing>=4.0.0 widens the floor below the previously pinned 4.3.1. If any code in this SDK relies on APIs added between 4.0.0 and 4.3.1, this could allow broken installs. Consider using >=4.3.1 to match the prior lower bound, which also aligns with the pattern used for web3 (where the new floor equals the old pin).

Copy link

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a straightforward and justified fix. Exact version pinning (==) in a library's pyproject.toml is generally an anti-pattern — it forces all downstream consumers to use exactly those versions, which causes conflicts on systems like Nix 24.05 that provide different versions. Relaxing to >= lower bounds is the right direction. One minor concern: using >= without an upper bound allows future major versions (e.g., web3 7.x, eth_typing 5.x) that may introduce breaking changes. The ~= compatible-release specifier would be slightly safer, but this is a style nit rather than a bug. The change is correct, improves compatibility, and has no security implications.

pyproject.toml (line 35): Consider using the compatible-release operator (~=) instead of >= to allow patch/minor updates while guarding against potentially breaking major-version bumps:

"eth_typing~=4.0",
"web3~=6.3",

~=4.0 is equivalent to >=4.0, <5.0, and ~=6.3 to >=6.3, <7. Since web3 itself constrains eth_typing transitively, this is a low-priority nit, but it would make the intent ("compatible with this major series") more explicit.

Copy link

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent is correct — strict exact-version pins prevent installation on systems like Nix 24.05 that may have slightly different versions available, and relaxing them is the right approach. However, the upper bound for web3 deserves attention: web3.py v7 introduced significant breaking API changes (e.g., middleware stack, provider interface), and web3>=6.3.0 with no ceiling will allow pip to resolve to v7, potentially breaking users silently. The eth_typing>=4.0.0 lower bound is also slightly looser than needed given the original pin was 4.3.1, though this is a minor concern.

pyproject.toml (line 36): Without an upper bound, this allows web3 v7+, which has breaking API changes (middleware stack, provider interface, and more) compared to the v6 series the SDK was developed against. Consider web3>=6.3.0,<7.0.0 until v7 compatibility is explicitly tested and confirmed.

pyproject.toml (line 35): The minimum was lowered from the original pin of 4.3.1 to 4.0.0. If the codebase relies on types or symbols introduced between 4.0.0 and 4.3.1, this could allow broken installs. Using >=4.3.0 would relax the constraint just enough to solve the Nix issue while staying closer to the tested minimum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BLUE This PR is simple and straightforward.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants