-
Notifications
You must be signed in to change notification settings - Fork 232
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: Add first draft of a Data Plane Signaling specification. #4331
Conversation
829f77d
to
e881786
Compare
Hey there, Based on our Discussion I tried to come up with what could be a first draft of a DPS specification. I took the IDSA DSP specification as an example. Would love your feedback on this one @jimmarino! |
This pull request is stale because it has been open for 7 days with no activity. |
Unfortunately, there seems to be a miscommunication: please do not write a specification. Documentation of the existing API would be appreciated, but a specification for general dataplane operations needs to be proposed as part of the EDWG and involves a lot of specialized knowledge. A specification is not the best place to start contributing to this project. Documentation, bug fixing, and discrete features are the recommended ways to learn the code base and gain the trust of committers. The existing documentation explains this. If you are set on writing a specification rather than documentation, I would recommend continuing to read my suggestions below and then consider if you want to become involved in one of the other EDWG projects. Regarding the technical specifics of this PR, the state machine described is incorrect and contains a number of inaccuracies. First, the state machine is maintained by the data plane; it is not a shared state machine between the control and data planes. Notifications are not part of the state machine and Also, a response message is a reply in a request-reply pattern and cannot be autonomously sent as it is, for example, in asynchronous patterns. This is a fundamental concept that is important to understand as it is the basis for the designed protocol and reliability guarantees. To put this more broadly, data plane signaling is not built using asynchronous message patterns but is request-reply. This statement (and others like it in the document) is not correct:
Please make sure you have a solid understanding of messaging patterns, the difference between request-reply and asynchronous variants, and how data plane signaling is implemented in the EDC. Also, please familiarize yourself with how documentation is created in EDC as this work should only be to cover existing behavior. Most importantly, please ask questions early on, not after a lot of work has been done, because I am sure it is frustrating to spend time on something that isn't accepted. |
This pull request is stale because it has been open for 7 days with no activity. |
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.
Some comments from @ronjaquensel and me
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.
We do not need actors in the sequence diagram. The control plane initiates the process, but this whole sequence is data plane internal. We would propose to remove the actors from the arrows.
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.
Please review the terminal states; the ones in the picture are not correct: COMPLETED
and TERMINATED
are the final states.
|
||
##### 2. `SUSPEND` | ||
![](data-plane-signaling-state-machine.png) |
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.
Having this picture in general makes sense.
|
||
##### 1. `START` |
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 original document structure was focussing on the flow, not the messages. A proposal would be to keep this document as it was and create a new one that focusses on EDC's implementation:
Possible title could be data-plane-mechanisms.md
Possible ToC:
- State Machine
- State
STARTED
- State
SUSPENDED
- State
TERMINATED
- State
COMPLETED
- etc.
For each state, it is described how the state is reached (e.g., via a message or data plane internal processes (i.a., in case of errors)), how the message looks like, what Http endpoint is provided and called, and some further comments (those can be adopted from this PR's content). There is no need to apply the structure of the DSP documents. This removes the "specification character". As stated in the discussion and the PR, we do NOT want to create a specification for that in any EDC repositories.
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.
how the message looks like
We would prefer inline JSON snippets of EDC messages, no .ttl
files or .puml
diagrams.
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.
WDYT? @jimmarino
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.
Agreed documentation is fine but not a specification. There are also issues that need to be corrected related to sync vs. async messaging and the state machine.
Why can't this be shifted to: How to implement a Data Plane? This can be described in a language-neutral way and focus on the signaling API that needs to be implemented. The emphasis would be on describing the HTTP endpoints that need to be implemented. The result would become part of the document revamp we are doing.
This pull request is stale because it has been open for 7 days with no activity. |
This pull request was closed because it has been inactive for 7 days since being marked as stale. |
What this PR changes/adds
This PR introduces an agnostic Data Plane Signaling (DPS) specification based on the IDSA Dataspace Protocol Specification template. This specification serves as a foundational document that can be further developed to create a comprehensive DPS specification by the Eclipse Dataspace Working Group (EDWG) at a later stage.
Why it does that
The current documentation for DPS is sparse and lacks sufficient detail for developers to effectively implement their own Data Planes. This PR aims to provide a more detailed and agnostic specification that ensures compatibility and effective communication between the Control Plane and Data Plane. It addresses the need for a well-defined protocol specification, which is crucial for broader adoption and interoperability.
Further notes
TransferProcessCompleteRequest
, which has been added in order to reach consistency by offering the respective counterpart to aTransferProcessFailRequest
.Linked Issue(s)
Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.