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

docs: Add first draft of a Data Plane Signaling specification. #4331

Closed

Conversation

f-zimmer
Copy link

@f-zimmer f-zimmer commented Jul 4, 2024

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

  • The specification is designed to be implementation-independent and does not include any implementation advice, or references to specific classes or packages. However, the first version was hardly influenced by the current implementation.
  • The specification includes descriptions of message types using JSON Schema, in line with the guidelines provided during the discussion.
  • This initial version serves as a starting point for further contributions and enhancements by the community and the EDWG.
  • The specification mentions a TransferProcessCompleteRequest, which has been added in order to reach consistency by offering the respective counterpart to a TransferProcessFailRequest.

Linked Issue(s)

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

@f-zimmer f-zimmer marked this pull request as ready for review July 4, 2024 09:40
@f-zimmer
Copy link
Author

f-zimmer commented Jul 4, 2024

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!

Copy link

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale Open for x days with no activity label Jul 12, 2024
@ndr-brt ndr-brt requested a review from jimmarino July 12, 2024 07:02
@ndr-brt ndr-brt removed the stale Open for x days with no activity label Jul 12, 2024
@jimmarino
Copy link
Contributor

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 suspended is not a terminal state.

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:

The response messages are either sent as a direct response to an incoming message or as an indirect response at a later stage during the transfer process

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.

Copy link

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale Open for x days with no activity label Jul 20, 2024
@juliapampus juliapampus removed the stale Open for x days with no activity label Jul 23, 2024
Copy link
Contributor

@juliapampus juliapampus left a 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

Copy link
Contributor

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.

Copy link
Contributor

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)
Copy link
Contributor

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`
Copy link
Contributor

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:

  1. State Machine
  2. State STARTED
  3. State SUSPENDED
  4. State TERMINATED
  5. State COMPLETED
  6. 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.

Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT? @jimmarino

Copy link
Contributor

@jimmarino jimmarino Jul 25, 2024

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.

Copy link

github-actions bot commented Aug 2, 2024

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale Open for x days with no activity label Aug 2, 2024
Copy link

github-actions bot commented Aug 9, 2024

This pull request was closed because it has been inactive for 7 days since being marked as stale.

@github-actions github-actions bot closed this Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Open for x days with no activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants