Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Greptile OverviewGreptile SummaryThese changes attempt to stabilize CI/package/docs builds after upstream Most updates are limited to CI workflows, Docker build steps, and Nox’s install check. One additional runtime dependency change ( Confidence Score: 3/5
Important Files Changed
|
| requests-oauth2client>=1.5.0 | ||
| requests-oauthlib==2.0.0 | ||
| rich-click==1.9.0 |
There was a problem hiding this comment.
Unrelated dependency bump
This PR’s stated scope is pinning setuptools/disabling build isolation to address pkg_resources build breakages, but it also bumps scylla-driver (3.26.8 → 3.29.7). That introduces a behavior/compatibility change at runtime that isn’t explained or tested here, and it makes it harder to reason about whether the CI fix is safe. If the scylla bump isn’t required for the setuptools issue, it should be reverted/split into a separate PR with its own rationale/test plan.
| [build-system] | ||
| requires = ["setuptools", "wheel", "versioneer-518"] # PEP 508 specifications. | ||
| requires = ["setuptools==80.10.2", "wheel", "versioneer-518"] # PEP 508 specifications. | ||
|
|
There was a problem hiding this comment.
Overly strict build pin
Pinning build-system.requires to an exact setuptools==80.10.2 can hard-fail source builds in environments where that exact wheel isn’t available (or Python compatibility differs), because PEP-517 frontends must satisfy this requirement before any build can proceed. For the stated goal (avoid newer setuptools that dropped pkg_resources), consider using a bounded range (e.g. <82) here and keep the exact pin in CI/Docker where the environment is controlled.
| # Install Python Dependencies | ||
| RUN pip --no-cache-dir --disable-pip-version-check install --upgrade pip setuptools wheel | ||
| RUN pip --no-cache-dir --disable-pip-version-check install --upgrade pip setuptools==80.10.2 wheel | ||
|
|
||
| COPY requirements.txt . | ||
| RUN pip install --no-cache-dir -r requirements.txt | ||
| RUN pip install --no-cache-dir --no-build-isolation -r requirements.txt | ||
| COPY optional-requirements.txt . | ||
| RUN pip install --no-cache-dir -r optional-requirements.txt | ||
| RUN pip install --no-cache-dir --no-build-isolation -r optional-requirements.txt | ||
|
|
There was a problem hiding this comment.
Pin may be overridden
The image pins setuptools==80.10.2 initially, but subsequent pip install ... -r requirements.txt/optional-requirements.txt/dev-requirements.txt can still cause pip to replace setuptools if any requirement set constrains it differently (or if pip decides to resolve differently). If the intent is to guarantee this version throughout the build, prefer enforcing it via a constraints file (e.g. pip install -c constraints.txt -r requirements.txt) rather than a one-off preinstall.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Additional Comments (1)
This installs |
thabofletcher
left a comment
There was a problem hiding this comment.
I dont think we should worry too much about greptiles comments, this LGTM once it passes
Unticketed
Description Of Changes
Fixes some build issues that started to make our CI fail due to upstream changes in
setuptoolsto no longer ship withpkg_resources: see okta/okta-sdk-python#496 and https://github.com/pypa/setuptools/blob/main/NEWS.rst#v8200 for more context. we don't usepkg_resourcesourselves directly, but some of our dependencies do, transitively.This is a temporary fix to get our CI unblocked. #7328 will be a much-needed, more holistic update to our build/packaging pipeline.
Code Changes
setuptoolsto80.10.2(which still includespkg_resources) in all of our build pipelinesno-build-isolationis set in all of our build/compile stages, to avoid some of our dependencies that build from source automatically using the most recentsetuptoolsin their isolated build environment and failing, e.g. https://github.com/ethyca/fides/actions/runs/21839395886/job/63019453233Steps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works