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

Add schema relationships diagram to dash-sonic-hld #94

Merged
merged 29 commits into from
Jun 13, 2022

Conversation

chrispsommers
Copy link
Collaborator

@chrispsommers chrispsommers commented Apr 13, 2022

I want to facilitate discussion of #93, in general how they relate to one another but in particular how are the JSON schema derived. My understanding is they could be derived directly from the DASH_APP_DB object schema using sonic-cfggen, thus we don't have to "invent" a schema by hand nor maintain yet another one.

# Canonical Test Data

### Figure - Schema Relationships

Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this diagram and the relationships it shows. Perhaps, it belongs in @prsunny document SONiC-DASH HLD when the details are ironed out which is the goal of the PR #93.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you Michael, that would be a good outcome. I needed to put it somewhere for review. I'm perfectly fine with the content being adapted/merged into #93 if it's useful, and I can close this PR. Or, I can delete the .md file from this PR, the image can remain and accepted into /images, then referred to in #93.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest keeping the 2 PRs open for now.
When we meet next time, and after some feedback hopefully, we can merge them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with @mmiele, this content is more on Sonic. You can update the Sonic HLD to have this details captured and update the diagram.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@prsunny would you please review again, I merged this into your dash-sonic-hld. I still need some clarity on the final JSON config format. Thanks.

@mmiele mmiele mentioned this pull request Apr 15, 2022
@chrispsommers chrispsommers changed the title Add schema relationships doc and figure to aid discussion of https://github.com/Azure/DASH/pull/92 Add schema relationships doc and figure to aid discussion of https://github.com/Azure/DASH/pull/93 May 13, 2022
@chrispsommers chrispsommers changed the title Add schema relationships doc and figure to aid discussion of https://github.com/Azure/DASH/pull/93 Add schema relationships diagram to dash-sonic-hld May 13, 2022
@KrisNey-MSFT KrisNey-MSFT requested a review from prsunny June 9, 2022 18:37
@KrisNey-MSFT KrisNey-MSFT merged commit a9ec96a into sonic-net:main Jun 13, 2022
@chrispsommers chrispsommers deleted the chris-add-schema-diagram branch June 13, 2022 16:35
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.

4 participants