Skip to content
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

Open
wants to merge 16 commits into
base: refac
Choose a base branch
from

Conversation

LadyCodesItBetter
Copy link
Collaborator

This PR introduces enhancements to the PresentationSubmission class to improve error handling and dynamic handler initialization based on descriptor_map.

Key points:

  • handlers are dynamically created for each descriptor_map entry based on the format key.
  • handlers attribute contains a dictionary where the key is the index of the descriptor, and the value is the respective handler instance.

Raises:
FileNotFoundError: If the configuration file is not found.
"""
config_path = os.path.join(os.path.dirname(__file__), "config.yml")
Copy link
Collaborator

@Zicchio Zicchio Jan 16, 2025

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.

Copy link
Member

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

Copy link
Collaborator

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.

Copy link
Member

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.

Copy link
Collaborator Author

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?

@Zicchio
Copy link
Collaborator

Zicchio commented Jan 16, 2025

handlers attribute contains a dictionary where the key is the index of the descriptor, and the value is the respective handler instance.

Sorry if the question is dull, but... what is an handler?
Let's say I create an instance of PresentationSubmission. How and when am I supposed to use self.handler? What should they do?

# Dynamically load the module and class
module = importlib.import_module(module_name)
cls = getattr(module, class_name)
handlers[index] = cls() # Instantiate the class
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

@LadyCodesItBetter LadyCodesItBetter marked this pull request as ready for review January 23, 2025 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define Presentation Submission Requirements for Verifiable Presentations
3 participants