Use referencing library where available instead of deprecated jsonschema.RefResolver #253
Conversation
martinhoyer
force-pushed
the
jsonschema
branch
from
689d45e
to
7c68e35
Compare
martinhoyer
changed the title
| @@ -34,6 +34,7 @@ BuildRequires: python%{python3_pkgversion}-jsonschema | |||
| BuildRequires: git-core | |||
| %{?python_provide:%python_provide python%{python3_pkgversion}-%{name}} | |||
| Requires: git-core | |||
| Recommends: python%{python3_pkgversion}-referencing | |||
There was a problem hiding this comment.
I don't think we need it. The dependency is part of jsonschema as early as 4.18 1 and for rhel case we can just assume it does not have the interface.
Footnotes
There was a problem hiding this comment.
oh cool, thanks. Removing.
|
|
||
| if MODERN_JSONSCHEMA: | ||
| # Modern approach with referencing | ||
| registry: Registry = Registry().with_resources([ |
There was a problem hiding this comment.
| registry: Registry = Registry().with_resources([ | |
| registry = Registry().with_resources([ |
Should not be necessary
| try: | ||
| from jsonschema import validators | ||
| from referencing import Registry, create | ||
| from referencing.jsonschema import DRAFT4 | ||
| MODERN_JSONSCHEMA = True | ||
| except ImportError: | ||
| MODERN_JSONSCHEMA = False |
There was a problem hiding this comment.
Could we make the test be on the importlib package version instead? It is less clear what MODERN_JSONSCHEMA is referring to otherwise. As for the create_validator, what about adding the imports in there?
| from __future__ import annotations | ||
|
|
||
| import warnings | ||
| from typing import Any, Dict, List, NamedTuple, Optional, Union |
There was a problem hiding this comment.
Let's not mix __future__.annotation and typing.Dict for now. We should take a hammer at it eventually though.
| class JsonSchemaValidationResult(NamedTuple): | ||
| """Represents JSON Schema validation result.""" | ||
| result: bool | ||
| errors: List[Any] |
There was a problem hiding this comment.
I don't think these should be in a _compat package. Maybe utils.jsonschema instead. For this PR though, we can keep them in the original place.
| with warnings.catch_warnings(): | ||
| warnings.filterwarnings('ignore', category=DeprecationWarning) |
There was a problem hiding this comment.
Let's try without the catch_warnings. There should not be the DeprecationWarning being triggered if the MODERN_JSONSCHEMA is working appropriately, and if it does, we should investigate why it does.
| def create_validator( | ||
| schema: Dict[str, Any], | ||
| schema_store: Optional[Dict[str, Any]] = None | ||
| ) -> Union[jsonschema.validators.Validator, jsonschema.validators.Draft4Validator]: | ||
| """Create a validator instance based on available jsonschema version.""" |
There was a problem hiding this comment.
I am more used to create naming convention being a static/classmethod. Just a thought if it should be get or create or something else.
| from fmf._compat_jsonschema import JsonSchemaValidationResult # noqa: F401 | ||
| from fmf._compat_jsonschema import validate_data # noqa: F401 |
There was a problem hiding this comment.
Why didn't fmf._compat.jsonschema work?
| @@ -419,7 +422,7 @@ class Logging: | |||
| _level = LOG_WARN | |||
|
|
|||
| # Already initialized loggers by their name | |||
| _loggers = dict() | |||
| _loggers: dict[str, logging.Logger] = dict() | |||
There was a problem hiding this comment.
This doesn't have __future__.annotations here so it should be using Dict instead here. But also let's deffer the change.
Addressing #194
The deprecation warnings everywhere are getting too annoying to ignore :)