-
Notifications
You must be signed in to change notification settings - Fork 14
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
#299-feature/presentation submission dynamic handler #314
base: refac
Are you sure you want to change the base?
#299-feature/presentation submission dynamic handler #314
Conversation
Co-authored-by: Giuseppe De Marco <demarcog83@gmail.com>
…om:LadyCodesItBetter/eudi-wallet-it-python into feature/italia#299-Presentation-Submission
… for descriptor_map
pyeudiw/openid4vp/presentation_submission/presentation_submission.py
Outdated
Show resolved
Hide resolved
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the intended propose is for a generic python package, not necessarly used in the iam proxy context
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.
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.
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.
yeah, absolutely. We cannot rely on the relative path of tha file distributed within a python package.
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.
To address the issue of accessibility and management of the config.yml
file, I propose two possible solutions to enhance flexibility and simplify the use of the library:
- dynamic configurations via env of configuration file (a mechanism to dynamically specify the configuration file path using an environment variable, such as
PYEUDIW_CONFIG_PATH
) - inline configuration support (allow configuration to be passed directly as a Python dictionary or JSON object during class initialization)
@peppelinux which do you think would work better for our use case?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
The PresentationSubmission class processes and validates presentation submissions. Its handlers attribute is a dictionary mapping indices from the descriptor_map in the submission to dynamically instantiated handler objects. These handlers are responsible for specific operations (e.g., validation or transformation) tied to the format defined in a configuration file (config.yml).
The handlers are not just namespaces for static methods. They are instances of dynamically loaded classes based on the descriptor formats, designed to process, validate, or manage specific data. Each handler can implement complex logic and maintain state, depending on the requirements of the associated format. This approach ensures modularity, configurability, and extensibility, far beyond the role of a simple placeholder.
This PR introduces enhancements to the PresentationSubmission class to improve error handling and dynamic handler initialization based on descriptor_map.
Key points: