-
Notifications
You must be signed in to change notification settings - Fork 238
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
docs: decision record about multiple protocol versions #4495
docs: decision record about multiple protocol versions #4495
Conversation
a3d38c6
to
841a17e
Compare
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.
just an idea... why not use the protocol
field that already exist?
currently the value is always dataspace-protocol-http
, it could become dataspace-protocol-http:version
, that will likely simplify the work:
- no need to change CN and TP entities
- no need to change
RemoteMessage
interface and relative implementations - no need to change
ContractRequest
andTransferRequest
all the rest looks good
wdyt?
I also though about this, it will probably reduce the work for supporting an additional fields everywhere. |
Probably by refactoring the @ExtensionPoint
public interface RemoteMessageDispatcherRegistry {
/**
* Registers a dispatcher.
*/
void register(String protocol, RemoteMessageDispatcher dispatcher);
} Could allow us to register the same |
we could refactor the dispatcher registry to being able to support the version (e.g. by split the otherwise also refactor the Maybe we are getting too technical here, but it's just a matter of refactoring |
i would try to stick to a single Having automatically split in the registry will work but it will always dispatch even of unsupported versions. In case we could put a check in the implementation underlying What about the refactoring I proposed above? in case i can amend the DR :) and go with the version in the |
looks good to me 👍 |
841a17e
to
4e58fb2
Compare
What this PR changes/adds
Decision record about multiple protocol versions
Why it does that
Briefly state why the change was necessary.
Further notes
List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.
Linked Issue(s)
Closes # <-- insert Issue number if one exists
Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.