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

Split the subject object into subject and predicate #51

Closed
e-backmark-ericsson opened this issue Jul 7, 2022 · 7 comments
Closed

Split the subject object into subject and predicate #51

e-backmark-ericsson opened this issue Jul 7, 2022 · 7 comments
Labels
roadmap Items on the roadmap to-be-considered
Milestone

Comments

@e-backmark-ericsson
Copy link
Contributor

e-backmark-ericsson commented Jul 7, 2022

I propose that the subject object within each event type definition is split into two objects - subject and predicate, where the new subject object would contain items that are identical for all event types on the same subject, and the predicate contains the attributes that are different for events with different predicates but same subject.

At the same time the "content" object could be collapsed I believe.

I think this provides a clearer structure of the event. A drawback with this approach is that it might sometimes be hard to determine whether a field is to be placed in the predicate object or the subject object. One such example is the url in the example below. It could be argued that it is the same for both predicates defined ("started" and "finished"), but I'd say it could be different since the "finished" event could link to an archived edition of the taskRun execution while the "started" event would link to an ongoing execution.

Example for the dev.cdevents.taskrun.finished event, that should be changed from:

{
  "context": {
     ...
  },
  "subject": {
    "id": "mySubject123",
    "source": "/event/source/123",
    "type": "taskRun",
    "content": {
      "taskName": "myTask",
      "url": "https://www.example.com/mySubject123",
      "pipelineRun": {
        "id": "mySubject123"
      },
      "outcome": "failure",
      "errors": "Something went wrong\nWith some more details"
    }
  }
}

to:

{
  "context": {
     ...
  },
  "subject": {
    "id": "mySubject123",
    "source": "/event/source/123",
    "type": "taskRun",
    "name": "myTask",
    "pipelineRun": {
      "id": "mySubject123"
    }
  },
  "predicate": {
    "type": "finished",
    "url": "https://www.example.com/mySubject123",
    "outcome": "failure",
    "errors": "Something went wrong\nWith some more details"
  }
}
@afrittoli
Copy link
Contributor

We should expand on the motive and benefits so we can decide when to implement this.

@afrittoli
Copy link
Contributor

The subject.id is the only place where the camel-case version of the subject is stored. If we remove it, we need to provide an alternative way for SDK generators to know about the structure of the subject (and potentially the predicate). It might be possible to annotate the jsonschema somehow with this info, to be verified.

@afrittoli
Copy link
Contributor

This change helps with human readability.
The way to split subject and predicate would be to look at fields which are common across the events today.
This is quite some to work to be implemented in the spec and all SDKs.

@xbcsmith
Copy link

xbcsmith commented Sep 9, 2024

I think the change improves the human readability and would help clear up the documentation. The change would be a large amount of work and would need to be coordinated with the SDKs.

@afrittoli
Copy link
Contributor

We can include this in hacktoberfest as a possible set of tasks.

@xibz
Copy link
Contributor

xibz commented Sep 30, 2024

Talked about this in today's SIG, while I agree it may improve some readability, I dont think it will be that tremendous of an improvement. The subject.changed nesting is only one level deep. The data structure for subject is quite simple. If the subject was much complicated I would tend to agree this improves readability. And with that said, it's important to note that machines do not care about the structure of the JSON, and given that it is still pretty readable, I'd be okay closing this.

@afrittoli @xbcsmith

@afrittoli
Copy link
Contributor

Closing for now.

@github-project-automation github-project-automation bot moved this from Todo to Done in CDEvents Releases Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
roadmap Items on the roadmap to-be-considered
Projects
Status: Done
Status: No status
Development

No branches or pull requests

4 participants