gs1 / EPCIS

Draft files being shared for EPCIS 2.0 development
Other
20 stars 7 forks source link

remove POST /eventTypes/{eventType}/events #308

Closed shalikasingh closed 3 years ago

shalikasingh commented 3 years ago

Hi @mgh128, @CraigRe,

As discussed have removed POST /eventTypes/{eventType}/events from openapi.yaml and deleted file EPCIS-JSON-Schema-single-event.json as it won't be required any longer. Please review and then can be merged.

FYI: @domguinard, @joelvogt

domguinard commented 3 years ago

Hi @shalikasingh. I'm not sure I understand what's the rationale to remove this endpoint? I did not see any discussion around that but sorry if I missed it.

Posting a single event that way is quite a useful pattern which was extensively discussed an agreed.

Cheers,

Dom

CraigRe commented 3 years ago

@shalikasingh , my recollection from yesterday (22 June)'s conversation is that we (re-)confirmed EPCIS 2.0 would not support EPCISMasterDataDocument or SimpleMasterDataQuery...

...and/but I am as confused as Dom as to why we would remove POST /eventTypes/{eventType}/events

?

dakbhavesh commented 3 years ago

Hi @CraigRe and @domguinard,

It was discussed during the 22 June call (only a few members were present as the call was extended and others had to leave) that two endpoints for capturing events lead to maintenance of multiple JSON schema. This came up while discussing observations in schema changes made by @jmcanterafonseca-iota as part of PR#298. @mgh128 mentioned that it will be against architectural guidelines to have duplicate functionality where /capture is sufficient to capture single or multiple events.

It was sort of an agreed decision to remove POST /eventTypes/{eventType}/events however if we want to keep it then @jmcanterafonseca-iota will have to make sure that schema for a single event must also be updated with new changes and tested thoroughly.

With two capture endpoints we have a trade-off of updating two schemas however having a single capture endpoint is nice to have as to make it RESTful.

@mgh128 - How should we proceed here?

CraigRe commented 3 years ago

@shalikasingh yes (sorry) -- that jars it back for me. I don't disagree with this approach; @mgh is AFAIK out of the loop since this afternoon, until late July. @domguinard concerns?

domguinard commented 3 years ago

@CraigRe @dakbhavesh @shalikasingh from what was discussed in the last call we are actually not removing these endpoints correct?

CraigRe commented 3 years ago

@domguinard yes, that is my understanding, but we agreed with the suggestion by @mgh128 & @dakbhavesh that a 2x2 table to assess pros/cons of both approaches would facilitate a WG decision on next week's WG call; googledocs link to follow (watch this space)...

dakbhavesh commented 3 years ago

Hi @CraigRe, Here is the phrasing to be placed under Section 10.1.3 Features of the JSON-LD context object.

Recommendation about the placement of JSON-LD @context:

Standard recommends placing consolidated @context with all default and user-defined namespaces at the root level of the EPCISDocument (refers to the payload of POST /capture endpoint) and at the root level of the EPCISEvent (refers to POST /event) endpoint. Having consolidated the @context object at the root level will reduce document parsing and validation complexity.

@mgh128 / @jmcanterafonseca-iota / @VladimirAlexiev - Feel free to update phrasing to make it more precise / meaningful if required.

CraigRe commented 3 years ago

Thanks for the quick turnaround, @dakbhavesh !

I've added the following, slightly tweaked text to § 10.1.3 of draft 07_20o, now in the community room:

EPCIS 2.0 capturing applications that use the JSON-LD context object SHOULD specify a consolidated @context for all default and user-defined namespaces at the root level of the EPCISDocument (refers to the payload of POST /capture endpoint) as well as at the root level of the EPCISEvent (refers to POST /event endpoint). This is strongly recommended, in order to simplify document parsing and validation.

VladimirAlexiev commented 3 years ago

I like it.

@CraigRe "as well as" -> "or" ?

@dakbhavesh can I take this chance to ask you to review EPCIS semantics? It also talks a bit about the context

CraigRe commented 3 years ago

@VladimirAlexiev "or" in place of "as well as" would not suffice if we are retaining both endpoints (i.e., for standalone EPCIS events, as well as those wrapped within an EPCIS Document), unless we make the explanation a bit more convoluted to address the following approaches:

  1. @context could be specified ONLY at the root level of the EPCIS Document, provided that events are wrapped within an EPCISDocument, rather than being captured as standalone EPCIS events, AND provided that EPCIS accessing applications using such data make provisions for client-side storage and retrieval/referencing of context data;

  2. alternatively, @context could be specified at the root level of each EPCIS event, potentially reiterating (effectively redundant) context info that might have been specified at the root level of the EPCIS Document, but in any case ensuring that the context info is not lost when individual events are parsed out of the EPCIS Document wrapper.

I will re-word the foregoing points less clumsily, assuming that the point of this passage is to ensure context info is not lost by accessing applications.

dakbhavesh commented 3 years ago

@dakbhavesh can I take this chance to ask you to review EPCIS semantics? It also talks a bit about the context

@VladimirAlexiev: I will surely review and let you know of any findings.

jmcanterafonseca-iota commented 3 years ago

Hi @CraigRe, Here is the phrasing to be placed under Section 10.1.3 Features of the JSON-LD context object.

Recommendation about the placement of JSON-LD @context:

Standard recommends placing consolidated @context with all default and user-defined namespaces at the root level of the EPCISDocument (refers to the payload of POST /capture endpoint) and at the root level of the EPCISEvent (refers to POST /event) endpoint. Having consolidated the @context object at the root level will reduce document parsing and validation complexity.

@mgh128 / @jmcanterafonseca-iota / @VladimirAlexiev - Feel free to update phrasing to make it more precise / meaningful if required.

I would add a starting subordinate, such as "Even though each individual member object in a JSON-LD document may declare its local @context, the EPCIS 2.0 standard recommends .... "

CraigRe commented 3 years ago

Hi @jmcanterafonseca-iota , I've added your subordinate clause lead-in to my previous revision:

Even though each individual member object in a JSON-LD document MAY declare its local @context, EPCIS 2.0 capturing applications that use the JSON-LD context object SHOULD specify a consolidated @context for all default and user-defined namespaces at the root level of the EPCISDocument (refers to the payload of POST /capture endpoint) as well as at the root level of the EPCISEvent (refers to POST /event endpoint). This is strongly recommended, in order to simplify document parsing and validation.

mgh128 commented 3 years ago

Sorry for the delay in preparing this table (as proposed on last week's call). Here's my attempt at the table: https://docs.google.com/document/d/12zNEwRlHUCE3GV_DmTMWEwhWF2gMZS9dp4pzcH-O-O4/edit?usp=sharing

I hope that it's consistent with the wording above and our current thinking - but we can adjust it further if necessary.

CraigRe commented 3 years ago

Discussed on earlier calls; re-affirmed on 17 August call.

PR 308 will now be closed, Mark's draft "where to declare JSON-LD context" will be added as a non-normative subsection to § 10 of the EPCIS draft.