sicpa-dlab / didcomm-python

basic DIDComm v2 support in python
Apache License 2.0
17 stars 15 forks source link

Bring Your Own Models (BYOM) #67

Closed dbluhm closed 1 year ago

dbluhm commented 1 year ago

I would like to recommend that we restructure the API of this library to accept standard objects (i.e. dictionary or perhaps dataclass?) wherever it is expecting a Message object. didcomm-python should then perform any necessary validation for the given operation but otherwise assume the data is well structured as received from the caller.

I make this recommendation for a few reasons:

This raises questions about what the return object of the unpack operation should be. I think there are plenty of examples out there where the caller specifies a class or a callable that can be used to deserialize the object.

I am not suggesting that the Message object should be removed altogether. If the user wants more structure than a dictionary but doesn't want to define it themselves, then Message would be available. Additionally, there's likely value in continuing to use it internally for the Forward message.

Open to thoughts.

cc @TelegramSam @chumbert @burdettadam

mwherman2000 commented 1 year ago

@dbluhm Not just a curiosity question: have you given any thought about when to use:

  1. a new DIDComm Message model, vs.
  2. a Verifiable Credential attached to an existing DIDComm Message type?

Michael

TelegramSam commented 1 year ago

@dbluhm Not just a curiosity question: have you given any thought about when to use:

  1. a new DIDComm Message model, vs.
  2. a Verifiable Credential attached to an existing DIDComm Message type?

Michael

The purposes of a DIDComm message and a Verifiable Credential are pretty darn different. I don't see any real confusion here. Can you explain further what you mean? @mwherman2000

TelegramSam commented 1 year ago

What about behaviors a message should have? Like automatic message IDs, the ability to generate a reply from a message matching thread ids, etc? Should we not have those in favor of the flexible model? Just teasing out the implications of what you are proposing.

dbluhm commented 1 year ago

I think message ID insertion could be done as part of validation during pack operations, if we wanted. Generating a reply message with threading I would leave to the caller.

dbluhm commented 1 year ago

On giving it a bit more thought, I think all message and thread ID convenience tweaks should belong as part of the model implementation and associated helpers. This does not invalidated #64; making the Message class included with the library easier to use is a goal I think we should keep.