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

Feature/metadata dtdl #32

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Feature/metadata dtdl #32

wants to merge 3 commits into from

Conversation

mayrmax
Copy link
Contributor

@mayrmax mayrmax commented Dec 6, 2021

No description provided.

DTDL/company.json Show resolved Hide resolved
DTDL/node.json Show resolved Hide resolved
DTDL/publication.json Outdated Show resolved Hide resolved
DTDL/publication.json Outdated Show resolved Hide resolved
DTDL/workflow_audit.json Outdated Show resolved Hide resolved
DTDL/publication.json Outdated Show resolved Hide resolved
},
{
"@type": "Property",
"name": "Kind",
Copy link
Contributor

Choose a reason for hiding this comment

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

Kind could be removed since denoted by concrete subtype of the twin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need the info so the other node will also have it in the future (with twin sync) the subtypes are only available on the owning side.

"contents": [
{
"@type": "Property",
"name": "Disabled",
Copy link
Contributor

Choose a reason for hiding this comment

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

Soft-delete flags are often only properties on the persisted backend properties since instances won't be served anymore at APIs after deletion.
Not sure how to handle this with twins though...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could add them to the $metadata and store them there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get the point. What's the issue with the property Disabled?

@@ -0,0 +1,28 @@
{
"@context": "dtmi:dtdl:context;2",
"@id": "dtmi:io:tributech:publication:public;1",
Copy link
Contributor

Choose a reason for hiding this comment

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

The properties Disabled and Requests are common with dtmi:io:tributech:publication:restricted;1 and should be moved to the base.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

request info should only be available on the owing node same for soft delete

Copy link
Contributor

Choose a reason for hiding this comment

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

@mayrmax @chlumper
Should we add OwingNode as a property on dtmi:io:tributech:publication:base;1 to resolve that issue? I think in general it would be nice to be able to interpret twins without the necessity of the context information of the "current" node.

DTDL/node.json Outdated
"name": "Publications",
"minMultiplicity": 0,
"maxMultiplicity": 100,
"target": "dtmi:io:tributech:publication;1",
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to either reference the new base publication or add two relationships for the concrete types.

"minMultiplicity": 0,
"maxMultiplicity": 1,
"target": "dtmi:io:tributech:node;1",
"displayName": "Initiator Node",
Copy link
Contributor

Choose a reason for hiding this comment

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

With this naming for the fields (InitiatorNode and TargetNode) it's not obvious for me how a flow A -> B would look like:

Request:

  • Initiator: B
  • Target: A ??
    Subscription:
  • Initiator: B
  • Target: A ??

Maybe something like Source and Target would be a better fit...

},
{
"@type": "Relationship",
"name": "RequestedStreams",
Copy link
Contributor

Choose a reason for hiding this comment

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

Streams could be moved to base for Request/Subscription since there is no difference at the moment.

Copy link
Contributor

@simonpfeifhofer simonpfeifhofer left a comment

Choose a reason for hiding this comment

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

Nice work!

"name": "Initiator",
"minMultiplicity": 0,
"maxMultiplicity": 1,
"target": "dtmi:io:tributech:user;1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could not find dtmi:io:tributech:user.

@@ -0,0 +1,28 @@
{
"@context": "dtmi:dtdl:context;2",
"@id": "dtmi:io:tributech:publication:public;1",
Copy link
Contributor

Choose a reason for hiding this comment

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

@mayrmax @chlumper
Should we add OwingNode as a property on dtmi:io:tributech:publication:base;1 to resolve that issue? I think in general it would be nice to be able to interpret twins without the necessity of the context information of the "current" node.

"contents": [
{
"@type": "Property",
"name": "Disabled",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get the point. What's the issue with the property Disabled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants