Remove FIDES_PBAC_LIB_PATH env var, add importlib fallback#8213
Open
galvana wants to merge 8 commits into
Open
Remove FIDES_PBAC_LIB_PATH env var, add importlib fallback#8213galvana wants to merge 8 commits into
galvana wants to merge 8 commits into
Conversation
…y discovery The libpbac shared library search now uses importlib.util.find_spec as a fallback when __file__ is a volume mount (e.g. fidesplus dev containers hot-reloading fides source). This eliminates the need for manual binary copying in docker-compose and the FIDES_PBAC_LIB_PATH env var. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8213 +/- ##
==========================================
- Coverage 85.62% 85.61% -0.01%
==========================================
Files 659 659
Lines 42898 42906 +8
Branches 5021 5023 +2
==========================================
+ Hits 36731 36734 +3
- Misses 5063 5066 +3
- Partials 1104 1106 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CI misc-unit runner doesn't have Go toolchain, so libpbac.so isn't present. Skip the two tests that need the real binary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The prod/test Dockerfile targets install fides from sdist into /opt/fides/ (non-editable). The compiled libpbac.so lives at /fides/src/fides/bin/ from the build stage but isn't included in the sdist. Copy it into the installed package's bin/ directory so find_library() can locate it. Also removes skipif from tests since the binary is now guaranteed present in all environments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CI test target mounts the repo checkout over /fides/, shadowing the libpbac.so that the Dockerfile built into /fides/src/fides/bin/. Copy the binary to /opt/fides/lib/ (not shadowed) and add it as a search path in find_library(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Different environments (editable install, Docker, site-packages) return different paths. What matters is the binary is found and real. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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.
Description Of Changes
Removes the
FIDES_PBAC_LIB_PATHenvironment variable and makes libpbac discovery fully automatic. When fides source is volume-mounted for hot-reload (e.g. in fidesplus dev containers),__file__points to the mount instead of site-packages, so the binary isn't found at the__file__-relative path. The newimportlib.util.find_spec("fides")fallback resolves this by locating the installed package regardless of__file__remapping.Code Changes
src/fides/service/pbac/engine.py: RemoveFIDES_PBAC_LIB_PATHenv var check, addimportlib.util.find_specfallback, rename_find_librarytofind_library, replace build hint error with searched-paths diagnostictests/service/pbac/test_engine_unit.py: Rewrite tests to verify real library discovery instead of mocking everything; test importlib path resolution, error diagnosticsSteps to Confirm
pytest tests/service/pbac/test_engine_unit.py-- all tests passfides-pkgmount, verifyfrom fides.service.pbac.engine import find_library; find_library()returns a valid pathPre-Merge Checklist
CHANGELOG.mdupdatedAdd a db-migration labelAdd a high-risk label