Add method to populate the cache of the schema manually #360
Conversation
…ly for unit testing)
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #360 +/- ##
===========================================
+ Coverage 69.50% 73.79% +4.28%
===========================================
Files 92 92
Lines 8202 8499 +297
Branches 1582 1664 +82
===========================================
+ Hits 5701 6272 +571
+ Misses 2082 1789 -293
- Partials 419 438 +19
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 14 files with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
LGTM. For the comments regarding client.timeout I think it's fine to deprecate those in an upcoming PR to as long as we do it a while before 2.0.
I think it could be useful to have some method to get the schema from the cache so that the users are returned a BranchSchema object that they'd then can store locally as ysml or yaml.
|
|
||
| Returns: | ||
| dict[str, MainSchemaTypes]: Dictionary of all schema organized by kind | ||
| """ | ||
| branch_schema = self._fetch(branch=branch, namespaces=namespaces, timeout=timeout) | ||
| if timeout: | ||
| self._deprecated_schema_timeout() |
There was a problem hiding this comment.
We should also deprecate timeout in the client.create() methods it's only used to pass the argument to the schema function.
There was a problem hiding this comment.
It's already covered because if someone use timeout while creating a node, it will generate a deprecation warning.
Having said that I've updated the changelog to reflect that properly
| def _deprecated_schema_timeout() -> None: | ||
| warnings.warn( | ||
| "The 'timeout' parameter is deprecated while fetching the schema and will be removed version 2.0.0 of the Infrahub Python SDK. " | ||
| "Use client.default_timeout instead.", |
There was a problem hiding this comment.
Will you also create an issue in the 2.0 milestone to remove this.
| branch = branch or self.client.default_branch | ||
|
|
||
| if isinstance(schema, dict): | ||
| schema = BranchSchema.from_api_response(data=schema) |
There was a problem hiding this comment.
In a way I'd prefer if .set_cache took a BranchSchema object directly. That way there's no confusion regarding what the dict[str, Any] should look like. It would mostly be relevant if there are any errors in the schema, then those errors would be raised in the correct location. On the other hand it might be slightly easier for some users if they don't have to convert it into a dict.
There was a problem hiding this comment.
BranchSchema is primarily an internal object so in most cases I would expect users to use the dict most of the time.
To clarify, I've added support for SchemaRootAPI as well which is the format returned by the /api/schema endpoint
Deploying infrahub-sdk-python with
|
| Latest commit: |
11fa9f8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e55ba8c3.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://dga-20250416-set-schema-cach.infrahub-sdk-python.pages.dev |
| @@ -102,6 +110,23 @@ def validate_data_against_schema(self, schema: MainSchemaTypesAPI, data: dict) - | |||
| message=f"{key} is not a valid value for {identifier}", | |||
| ) | |||
|
|
|||
| def set_cache(self, schema: dict[str, Any] | SchemaRootAPI | BranchSchema, branch: str | None = None) -> None: | |||
There was a problem hiding this comment.
I think set_cache should be a private method as it's meant for internal use.
There was a problem hiding this comment.
it's not meant for internal use, it's meant for user to use during unit tests
Fixes #342
This PR mainly add a method to manually populate the cache for the schema manager, the primary use case for this method is to simplify the initialization of the client during unit tests
I also deprecated the timeout parameter while fetching the schema, it was not consistently used within the code and overall I think using the default timeout should be enough for the schema.
I'll open another issue shortly to track its removal