Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Provide non-normative examples of state transitions and adjust state description #138
base: main
Are you sure you want to change the base?
Provide non-normative examples of state transitions and adjust state description #138
Changes from 2 commits
d8c3b3a
427957a
96bb0f8
0b829b6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Too specific. We should never name components in this specs, like "central cataloging system", not even in non-normative documents. In addition, this is a wrong statement. This transition indeed can be base on previous interactions between consumer & provider.
With this we leave it open:
or
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.
I think it should be "The consumer requests a contract based for a dataset" since "a previously requested dataset or catalog" is not defined.
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.
As we want to give the reader an idea, that such dataset can be identified out of scope of the procool, I suggest: "The consumer requests a contract for a data set that has been brought to his attention outside the scope of the Protocol."
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.
That's also not entirely accurate. The consumer could make up a contract offer for an existing data set. These types of explanations are best done in a separate, non-normative document. Examples like this are valuable but should not be in this document. Per my comment below about the role of specifications, they have no bearing on how an implementor must ensure correct protocol behavior.
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.
You state "without previous interaction" and in the same sentence name business relationships.
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.
This should read "The provider sends a contract offer to a consumer" as an "existing business relationship" is undefined.
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 point is, that we add the table especially for a more explanatory non-normative content. Stripping away all examples, brings the table back to a normative state and gets obsolete, as the diagram explains the transitions quite well. Thus, I would prefer @juliapampus suggestion
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.
I don't see how that is more explanatory, as "context" and "existing business relationship" are vague terms. What is a "business relationship," and what is a "business context"? Those are completely open-ended.
The purpose of the specification is not to explain how DSP may be used. IMO, the specification should explain to implementors what the correct protocol behaviors are and stop there. It is a normative document, not an explanation of how to implement a dataspace or how dataspace technologies may solve project requirements.
Those are fine goals, but they should be addressed in "architecture/primer/whitepaper" documents.
I also disagree with including the following types of statements:
Besides being an implementation detail, the DSP catalog protocol is designed to enable decentralized catalog designs (it mentions this and explicitly does not define a replication protocol).
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.
Naming
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.
Two things related things. "Counter" is not defined. I don't think we need "based on a previous contract request" because that is also ambiguous ("based on" is not defined). I would simply state "The provider sends a contract offer." The contract offer by definition is in the context of the contract negotiation but we could also state it explicitly, "The provider sends a contract offer for the referenced contract negotiation"
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.
@jimmarino
I would add "[...] contract negotiation process", to have a textual linking to the
processId
.@juliapampus
We might not have a dataset here, since INIT -> OFFERED is possible. So in general: How can the consumer decide, if the offer shall be accepted without a reference to a dataset? A
ContractOfferMessage
just hasprocessId
,offer
andcallbackAddress
. Is it possible that this edge may not be effective?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 spec should never make any such statements regarding public data, terms or laws.
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.
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.
For curiosity (maybe discuss next week): How could a consumer define other parameters inside the ContractRequestMessage? WIthin the
odrl:Offer
?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.
TERMINATED
andFINALIZED
are never initial states. The table may contain the additional transitions introduced in #43 either in the same rows than the success states, or new ones.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 most left column is the origin state, not an initial state. With
*/{...}
I wanted to express "All but {...}" from set theory, but I mixed up the backslash. Thus, the correct notation would be* \ {TERMINATED, FINALIZED}