#299-feature/presentation submission dynamic handler #314
Conversation
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| class PresentationSubmission: |
There was a problem hiding this comment.
don't put the classes within the init file
we are going to refactor all the code within this project to remove this code-style in favour or a proper division of files by scopes
Co-authored-by: Giuseppe De Marco <[email protected]>
…om:LadyCodesItBetter/eudi-wallet-it-python into feature/italia#299-Presentation-Submission
… for descriptor_map
| from pydantic import ValidationError | ||
| import yaml | ||
| import importlib | ||
| from typing import Dict, Any |
There was a problem hiding this comment.
Importing Dict is deprecated since Python 3.9 and this project officially supports Python 3.10+ only - use dict instead (no import required).
https://docs.python.org/3/library/typing.html#aliases-to-built-in-types
There was a problem hiding this comment.
go ahead using dict without any import
| Raises: | ||
| FileNotFoundError: If the configuration file is not found. | ||
| """ | ||
| config_path = os.path.join(os.path.dirname(__file__), "config.yml") |
There was a problem hiding this comment.
What's the point of having a class configurable by editing an obscure configuration file in the installed class project that is available who-knows-where based on what pip (or other build tool) is doing and might possibly be inside a container? Just use a config.py file at this point - it is functionally the same thing.
@peppelinux @LadyCodesItBetter I'm not sure here what is the intended design goal.
There was a problem hiding this comment.
the intended propose is for a generic python package, not necessarly used in the iam proxy context
There was a problem hiding this comment.
The point still stands. Assume I am using the project as a package, that is, I am using it with pip install pyeudiw as a part of my project.
To change the configuration of this class, I have to go in the location where pip placed config.yaml (which is usually inside the site-packages) and and edit it. This is IMO very convoluted and only doable by technically advanced users.
Sorry if the question is dull, but... what is an handler? |
| # Dynamically load the module and class | ||
| module = importlib.import_module(module_name) | ||
| cls = getattr(module, class_name) | ||
| handlers[index] = cls() # Instantiate the class |
There was a problem hiding this comment.
AFAIK the fact that each handler is a class initialized with an empty constructor implies that handlers are functionally just namespace for static methods. It's impossible to known what they should do without doing introspection on what aforementioned namespace contains.
| @@ -0,0 +1,121 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
maybe I'm wrong but if a test file name does not start with test_ or ends with _test.py then pytest will NOT execute it for unit tests (altough ci-cd pipeline might change this behaviour - I'm not 100% sure how unit tests are configured here).
In doubt, this file should be named test_presentation_submission.py
https://docs.pytest.org/en/stable/explanation/goodpractices.html#test-discovery
There was a problem hiding this comment.
I can confirm that _test doesn't get loaded
| class PresentationSubmissionSchema(BaseModel): | ||
| id: str | ||
| definition_id: str | ||
| descriptor_map: List[DescriptorSchema] |
There was a problem hiding this comment.
List is also deprecated: use list instead
There was a problem hiding this comment.
go ahead using list without any import
This PR introduces enhancements to the PresentationSubmission class to improve error handling and dynamic handler initialization based on descriptor_map.
Key points: