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

[V2] Set apiVersion to optional in normal DTOs, except for requestDTOs and EventDTO #537

Closed
weichou1229 opened this issue Mar 9, 2021 · 5 comments · Fixed by #538 or #546
Closed

Comments

@weichou1229
Copy link
Member

weichou1229 commented Mar 9, 2021

Per working group decision

Versionable -required on request/response objects and Event (because App Service can receive just an Event) DTO; for other objects, version is optional

see https://wiki.edgexfoundry.org/display/FA/Core+Working+Group?preview=/329472/60784641/CoreWG-AgendaAndMinutes-3-3-2021.pdf

  • Remove the required validation tag from apiVersion.
  • Check the requestDTOs and EventDTO which should contains valid api version
weichou1229 added a commit to weichou1229/go-mod-core-contracts that referenced this issue Mar 9, 2021
- Remove the required validation tag from apiVersion.
- Check the requestDTOs and EventDTO which should contains valid api version

Close edgexfoundry#537

Signed-off-by: weichou <[email protected]>
@weichou1229 weichou1229 changed the title Set apiVersion to optional in normal DTOs, except for requestDTOs and EventDTO [V2] Set apiVersion to optional in normal DTOs, except for requestDTOs and EventDTO Mar 9, 2021
@lenny-goodell
Copy link
Member

Now that we have had some time to see how this is working, maybe best to focus on having ApiVersion only on top level DTOs (i.e. Request/Response DTOs) and sub-DTOS that can be received by client w/o a parent Request/Response DTO (i.e. Event DTO). The other DTOs are just composite DTOs, so as we have seen it is redundant to have ApiVersion if the a parent DTO already has it.

So my suggestion now is to remove ApiVersion from DTOs that are not one of the following?

  • Request
  • Response
  • Event

@cloudxxx8 , @jpwhitemn Thoughts?

@cloudxxx8
Copy link
Member

@lenny-intel I would like to also re-visit the reason we keep ApiVersion in Event.
May we always use AddEventRequest when sending Event to App Service in V2?

@lenny-goodell
Copy link
Member

@cloudxxx8 , the Event DTO is what is exported to external destinations. Having the ApiVersion on Event is important if that end point has to support existing V1 deployments as the V2 deployments come on-line.

@cloudxxx8
Copy link
Member

@lenny-intel I understand now. In this case, we can implement it as your suggestion.

@weichou1229
Copy link
Member Author

@lenny-intel I removed the ApiVersion from normal DTOs except event and reading. Could you help review the PR #538?

weichou1229 added a commit to weichou1229/go-mod-core-contracts that referenced this issue Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants