-
Notifications
You must be signed in to change notification settings - Fork 6
feat: IHS-190 (part of IFC-2162) Add namespace restriction parameter to generic schemas #807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: infrahub-develop
Are you sure you want to change the base?
feat: IHS-190 (part of IFC-2162) Add namespace restriction parameter to generic schemas #807
Conversation
…ed_namespaces but also missing fields IHS-190
…rough infrahubctl command layer. Infrahub API is mocked IHS-190
…nvalid namespace is loaded from SDK methods. Infrahub API is mocked IHS-190
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## infrahub-develop #807 +/- ##
====================================================
- Coverage 80.33% 80.33% -0.01%
====================================================
Files 115 115
Lines 9865 9869 +4
Branches 1504 1504
====================================================
+ Hits 7925 7928 +3
- Misses 1419 1420 +1
Partials 521 521
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Small comment on this PR. It should target the |
WalkthroughThis pull request extends the generic schema API with support for namespace restrictions on generic types. Two new optional fields are added to the GenericSchemaAPI class: 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@infrahub_sdk/schema/main.py`:
- Around line 292-294: GenericSchema is missing the hierarchical and
restricted_namespaces attributes present in GenericSchemaAPI, causing those
values (e.g., restricted_namespaces from valid_generic_schema.json) to be
dropped when parsed via SchemaRoot; add the two fields to GenericSchema by
declaring hierarchical: bool | None = None and restricted_namespaces: list[str]
| None = None on the GenericSchema dataclass/model so it matches
GenericSchemaAPI and preserves round-trip parsing of those properties.
🧹 Nitpick comments (2)
tests/unit/sdk/test_schema.py (2)
516-537: Variableerror_messageis shadowed on Line 536.
error_messageis first defined on Line 516 as the mock response string, then reassigned on Line 536 to the extracted value from the response. This shadowing is confusing — use a distinct name for one of them.Suggested fix
- error_message = response.errors["errors"][0]["message"] - assert re.search(r"(?s)restricted namespaces(?=.*Dog)(?=.*Cat)", error_message) + actual_error = response.errors["errors"][0]["message"] + assert re.search(r"(?s)restricted namespaces(?=.*Dog)(?=.*Cat)", actual_error)
488-538: Test only covers the async path; consider adding sync coverage.Existing tests in this file consistently parametrize over
["standard", "sync"]client types (e.g., Lines 47, 66, 93). This test only exercisesInfrahubClient(async). Consider adding a sync variant usingInfrahubClientSync.schema.loadfor parity, consistent with the existing pattern in this file. As per coding guidelines,infrahub_sdk/**/*.py: Follow async/sync dual pattern for new features in the Python SDK.
| hierarchical: bool | None = None | ||
| used_by: list[str] = Field(default_factory=list) | ||
| restricted_namespaces: list[str] | None = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the structure around lines 283-294 in infrahub_sdk/schema/main.py
head -300 infrahub_sdk/schema/main.py | tail -50Repository: opsmill/infrahub-sdk-python
Length of output: 1709
🏁 Script executed:
# Also check the entire GenericSchema and GenericSchemaAPI definitions with more context
sed -n '280,310p' infrahub_sdk/schema/main.pyRepository: opsmill/infrahub-sdk-python
Length of output: 1092
🏁 Script executed:
# Find the SchemaRoot definition
rg -n "class SchemaRoot" --type=py -A 20Repository: opsmill/infrahub-sdk-python
Length of output: 2325
🏁 Script executed:
# Search for valid_generic_schema.json and how it's used
rg -n "valid_generic_schema" --type=py -B 2 -A 2Repository: opsmill/infrahub-sdk-python
Length of output: 715
🏁 Script executed:
# Check the fixture file to see if it contains the fields in question
cat tests/unit/fixtures/models/valid_generic_schema.jsonRepository: opsmill/infrahub-sdk-python
Length of output: 155
🏁 Script executed:
# Check the BaseModel/ConfigDict configuration for GenericSchema
sed -n '250,290p' infrahub_sdk/schema/main.py | grep -A 10 "class BaseSchema"Repository: opsmill/infrahub-sdk-python
Length of output: 398
🏁 Script executed:
# Also check BaseSchemaAttrRel to see if it has any of these fields
rg -n "class BaseSchemaAttrRel" --type=py -A 30Repository: opsmill/infrahub-sdk-python
Length of output: 2611
🏁 Script executed:
# Find the fixture file
find . -name "valid_generic_schema.json" -type fRepository: opsmill/infrahub-sdk-python
Length of output: 120
🏁 Script executed:
# Check if there are any fixture files in the test directory
find tests -name "*generic_schema*" -type fRepository: opsmill/infrahub-sdk-python
Length of output: 118
🏁 Script executed:
# Search for how ConfigDict is configured in the schema classes, especially looking for extra field behavior
rg -n "ConfigDict" --type=py infrahub_sdk/schema/main.pyRepository: opsmill/infrahub-sdk-python
Length of output: 625
🏁 Script executed:
# Check if convert_api() is called anywhere and how the returned GenericSchemaAPI is used
rg -n "convert_api" --type=py -B 2 -A 2Repository: opsmill/infrahub-sdk-python
Length of output: 4473
🏁 Script executed:
# Read the fixture file to see if it contains restricted_namespaces
cat tests/fixtures/models/valid_generic_schema.jsonRepository: opsmill/infrahub-sdk-python
Length of output: 737
🏁 Script executed:
# Check if there's a comment or test related to the fields not being in GenericSchema
rg -n "hierarchy|hierarchical|restricted_namespaces" --type=py tests/unit/sdk/test_hierarchical_nodes.py -B 2 -A 2Repository: opsmill/infrahub-sdk-python
Length of output: 15108
🏁 Script executed:
# Check the full ConfigDict settings for GenericSchema and GenericSchemaAPI to see if there's any special config
sed -n '283,300p' infrahub_sdk/schema/main.pyRepository: opsmill/infrahub-sdk-python
Length of output: 722
🏁 Script executed:
# Check how the fixture is actually used in tests
rg -n "valid_generic_schema" --type=py -B 5 -A 10Repository: opsmill/infrahub-sdk-python
Length of output: 1540
🏁 Script executed:
# Check if SchemaRoot parsing with the fixture would raise a validation error or silently drop fields
# Look at the test to see what happens
sed -n '133,160p' tests/unit/ctl/test_schema_app.pyRepository: opsmill/infrahub-sdk-python
Length of output: 1047
🏁 Script executed:
# Check if there are any schema validation tests that parse the fixture through SchemaRoot
rg -n "SchemaRoot.*valid_generic_schema" --type=py -B 5 -A 5Repository: opsmill/infrahub-sdk-python
Length of output: 53
🏁 Script executed:
# Search for how restricted_namespaces is used in the codebase
rg -n "restricted_namespaces" --type=py -B 2 -A 2Repository: opsmill/infrahub-sdk-python
Length of output: 2111
🏁 Script executed:
# Check Pydantic configuration in BaseModel and BaseSchema to see if extra fields raise errors
rg -n "model_config|extra|forbid" --type=py infrahub_sdk/schema/main.py | head -20Repository: opsmill/infrahub-sdk-python
Length of output: 573
🏁 Script executed:
# Check if the CLI actually parses the JSON through SchemaRoot or sends it directly
rg -n "load.*schema" --type=py infrahub_sdk/cli -B 5 -A 10 | head -100Repository: opsmill/infrahub-sdk-python
Length of output: 127
🏁 Script executed:
# Look at the complete test to see what happens with the loaded schema
sed -n '133,180p' tests/unit/ctl/test_schema_app.pyRepository: opsmill/infrahub-sdk-python
Length of output: 1713
🏁 Script executed:
# Check if there's any code that creates GenericSchema from JSON data that includes restricted_namespaces
rg -n "GenericSchema\(" --type=py -B 3 -A 3Repository: opsmill/infrahub-sdk-python
Length of output: 1013
🏁 Script executed:
# Check if there's a test that validates round-trip conversion of GenericSchema with restricted_namespaces
rg -n "test.*generic.*schema|test.*restricted_namespaces" --type=py -iRepository: opsmill/infrahub-sdk-python
Length of output: 565
🏁 Script executed:
# Look at the schema initialization/parsing logic to understand the flow better
rg -n "SchemaRoot\(" --type=py -B 3 -A 3Repository: opsmill/infrahub-sdk-python
Length of output: 2766
🏁 Script executed:
# Check if hierarchical is ever used as an input field or only as output
rg -n "hierarchical" --type=py -B 2 -A 2Repository: opsmill/infrahub-sdk-python
Length of output: 48353
🏁 Script executed:
# Check the Pydantic configuration more thoroughly - does it forbid extra fields?
sed -n '96,105p' infrahub_sdk/schema/main.pyRepository: opsmill/infrahub-sdk-python
Length of output: 358
🏁 Script executed:
# Let's trace through what happens when SchemaRoot parses the fixture with restricted_namespaces
python3 <<'EOF'
import sys
sys.path.insert(0, '.')
from infrahub_sdk.schema.main import SchemaRoot, GenericSchema
import json
# Load the fixture
with open('tests/fixtures/models/valid_generic_schema.json') as f:
fixture_data = json.load(f)
print("Fixture data:")
print(json.dumps(fixture_data, indent=2))
# Try to parse it through SchemaRoot
try:
schema_root = SchemaRoot(**fixture_data)
print("\nSchemaRoot parsing succeeded")
print(f"Number of generics: {len(schema_root.generics)}")
if schema_root.generics:
generic = schema_root.generics[0]
print(f"Generic name: {generic.name}")
print(f"Generic model dump: {generic.model_dump()}")
print(f"Does generic have restricted_namespaces? {hasattr(generic, 'restricted_namespaces')}")
except Exception as e:
print(f"Error parsing: {e}")
EOFRepository: opsmill/infrahub-sdk-python
Length of output: 411
🏁 Script executed:
pip install ujson pydantic -q && python3 <<'EOF'
import sys
sys.path.insert(0, '.')
from infrahub_sdk.schema.main import SchemaRoot, GenericSchema
import json
# Load the fixture
with open('tests/fixtures/models/valid_generic_schema.json') as f:
fixture_data = json.load(f)
print("Fixture data:")
print(json.dumps(fixture_data, indent=2))
# Try to parse it through SchemaRoot
try:
schema_root = SchemaRoot(**fixture_data)
print("\nSchemaRoot parsing succeeded")
print(f"Number of generics: {len(schema_root.generics)}")
if schema_root.generics:
generic = schema_root.generics[0]
print(f"Generic name: {generic.name}")
print(f"Generic model dump: {generic.model_dump()}")
print(f"Does generic have restricted_namespaces attr? {hasattr(generic, 'restricted_namespaces')}")
# Try convert_api
print("\nTrying convert_api()...")
api = generic.convert_api()
print(f"GenericSchemaAPI model dump: {api.model_dump()}")
print(f"Does API have restricted_namespaces? {api.restricted_namespaces}")
except Exception as e:
print(f"Error parsing: {type(e).__name__}: {e}")
EOFRepository: opsmill/infrahub-sdk-python
Length of output: 628
🏁 Script executed:
pip install ujson pydantic graphql-core -q && python3 <<'EOF'
import sys
sys.path.insert(0, '.')
from infrahub_sdk.schema.main import SchemaRoot, GenericSchema, GenericSchemaAPI
import json
# Load the fixture
with open('tests/fixtures/models/valid_generic_schema.json') as f:
fixture_data = json.load(f)
print("=== Fixture data ===")
print(json.dumps(fixture_data['generics'][0], indent=2))
# Try to parse it through SchemaRoot
print("\n=== Parsing through SchemaRoot ===")
try:
schema_root = SchemaRoot(**fixture_data)
print(f"✓ SchemaRoot parsing succeeded")
print(f" Number of generics: {len(schema_root.generics)}")
if schema_root.generics:
generic = schema_root.generics[0]
print(f" Generic name: {generic.name}")
dump = generic.model_dump()
print(f" Keys in model_dump(): {list(dump.keys())}")
print(f" Has 'restricted_namespaces' key? {'restricted_namespaces' in dump}")
# Try convert_api
print("\n=== Calling convert_api() ===")
api = generic.convert_api()
api_dump = api.model_dump()
print(f" GenericSchemaAPI keys: {list(api_dump.keys())}")
print(f" restricted_namespaces value: {api.restricted_namespaces}")
print(f" hierarchical value: {api.hierarchical}")
except Exception as e:
import traceback
print(f"✗ Error: {type(e).__name__}: {e}")
traceback.print_exc()
EOFRepository: opsmill/infrahub-sdk-python
Length of output: 767
🏁 Script executed:
pip install pydantic -q && python3 <<'EOF'
import sys
sys.path.insert(0, '.')
# Just import the schema module directly
from infrahub_sdk.schema.main import SchemaRoot, GenericSchema, GenericSchemaAPI
import json
# Load the fixture
with open('tests/fixtures/models/valid_generic_schema.json') as f:
fixture_data = json.load(f)
print("=== Fixture data ===")
print(json.dumps(fixture_data['generics'][0], indent=2))
# Try to parse it through SchemaRoot
print("\n=== Parsing through SchemaRoot ===")
try:
schema_root = SchemaRoot(**fixture_data)
print(f"✓ SchemaRoot parsing succeeded")
print(f" Number of generics: {len(schema_root.generics)}")
if schema_root.generics:
generic = schema_root.generics[0]
print(f" Generic name: {generic.name}")
dump = generic.model_dump()
print(f" Keys in model_dump(): {sorted(list(dump.keys()))}")
print(f" Has 'restricted_namespaces' key? {'restricted_namespaces' in dump}")
# Try convert_api
print("\n=== Calling convert_api() ===")
api = generic.convert_api()
api_dump = api.model_dump()
print(f" GenericSchemaAPI keys: {sorted(list(api_dump.keys()))}")
print(f" restricted_namespaces value: {api.restricted_namespaces}")
print(f" hierarchical value: {api.hierarchical}")
except Exception as e:
import traceback
print(f"✗ Error: {type(e).__name__}: {e}")
traceback.print_exc()
EOFRepository: opsmill/infrahub-sdk-python
Length of output: 767
🏁 Script executed:
# Check Pydantic's default behavior for extra fields in v2
sed -n '1,20p' infrahub_sdk/schema/main.pyRepository: opsmill/infrahub-sdk-python
Length of output: 520
🏁 Script executed:
# Check if there's any validation or extra field handling
rg -n "ConfigDict" infrahub_sdk/schema/main.py | head -20Repository: opsmill/infrahub-sdk-python
Length of output: 625
🏁 Script executed:
# Check if fields are validated elsewhere
rg -n "class GenericSchema" --type=py -A 10Repository: opsmill/infrahub-sdk-python
Length of output: 1175
🏁 Script executed:
# Check all parent classes to be sure neither BaseSchema nor BaseSchemaAttrRel define these fields
rg -n "class BaseSchema\|class BaseSchemaAttrRel" --type=py -A 20Repository: opsmill/infrahub-sdk-python
Length of output: 53
🏁 Script executed:
# Get BaseSchema and BaseSchemaAttrRel definitions
sed -n '260,290p' infrahub_sdk/schema/main.pyRepository: opsmill/infrahub-sdk-python
Length of output: 1080
🏁 Script executed:
# Get BaseSchemaAttrRel definition
sed -n '154,162p' infrahub_sdk/schema/main.pyRepository: opsmill/infrahub-sdk-python
Length of output: 438
🏁 Script executed:
# Check if there's any test that tries to create GenericSchema with restricted_namespaces
rg -n "GenericSchema.*restricted" --type=pyRepository: opsmill/infrahub-sdk-python
Length of output: 53
🏁 Script executed:
# Check if the issue is actually about API response vs input - search for where GenericSchemaAPI is used
rg -n "GenericSchemaAPI" --type=py | head -20Repository: opsmill/infrahub-sdk-python
Length of output: 2049
🏁 Script executed:
# Check how the CLI actually handles schema loading to understand if it goes through GenericSchema
sed -n '375,390p' infrahub_sdk/ctl/cli_commands.pyRepository: opsmill/infrahub-sdk-python
Length of output: 677
🏁 Script executed:
# Check if there's a conversion step that uses GenericSchema.convert_api()
rg -n "\.convert_api()" --type=py -B 3 -A 3 | grep -A 3 -B 3 "Generic"Repository: opsmill/infrahub-sdk-python
Length of output: 462
🏁 Script executed:
# Check the test to see if the raw JSON or parsed objects are sent to the API
sed -n '150,175p' tests/unit/ctl/test_schema_app.pyRepository: opsmill/infrahub-sdk-python
Length of output: 967
Add hierarchical and restricted_namespaces fields to GenericSchema.
GenericSchema lacks both fields that exist in GenericSchemaAPI, causing them to be silently dropped when schemas are parsed through SchemaRoot. The fixture valid_generic_schema.json includes restricted_namespaces: ["Dog"], which should be preserved. Add:
hierarchical: bool | None = None
restricted_namespaces: list[str] | None = Noneto GenericSchema to match its API counterpart and ensure consistent round-tripping.
🤖 Prompt for AI Agents
In `@infrahub_sdk/schema/main.py` around lines 292 - 294, GenericSchema is missing
the hierarchical and restricted_namespaces attributes present in
GenericSchemaAPI, causing those values (e.g., restricted_namespaces from
valid_generic_schema.json) to be dropped when parsed via SchemaRoot; add the two
fields to GenericSchema by declaring hierarchical: bool | None = None and
restricted_namespaces: list[str] | None = None on the GenericSchema
dataclass/model so it matches GenericSchemaAPI and preserves round-trip parsing
of those properties.
To understand the global feature, see this PR: opsmill/infrahub#8319
This PR is about updating
infrahub_sdk.schema.main.GenericSchemaAPIwith the new generic schema parameter "restricted_namespaces".Goal
See opsmill/infrahub#8319
Non-goals
Added another missing attribute to
infrahub_sdk.schema.main.GenericSchemaAPI: hierarchical. This attribute was already into the internal Infrahub generic schema model.What changed
Generic schemas into SDK API.
How-to review
This PR is quite small: commit by commit, in the chronological order.
How to test
Automated back-end tests:
uv run pytest tests/unit/ctl/test_schema_app.py tests/unit/sdk/test_schema.pyApplication testing scenarios:
Impact & rollout
Deployment notes: safe to deploy
Closes IHS-190
Summary by CodeRabbit
New Features
Tests