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

[server]fix missing services attribute for empty design #10968

Merged
merged 2 commits into from
May 15, 2024

Conversation

MUzairS15
Copy link
Contributor

Notes for Reviewers
The Services struct had omitempty tag because of which if the design was empty, it was not being streamed inside the response.

Signed commits

  • Yes, I signed my commits.

Signed-off-by: MUzairS15 <muzair.shaikh810@gmail.com>
@MUzairS15
Copy link
Contributor Author

// @aabidsofi19 @leecalcote

@MUzairS15 MUzairS15 merged commit 7818e6f into meshery:master May 15, 2024
11 checks passed
Copy link

github-actions bot commented May 15, 2024

Copy link
Member

@leecalcote leecalcote left a comment

Choose a reason for hiding this comment

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

Removal of this also leaves the system vulnerable to a nil primary key inside the design, @MUzairS15

@MUzairS15
Copy link
Contributor Author

The services key as you said should be ever present, and this PR ensures this happens.
Comparing it with primary constraint not null, if design is empty, it is not that services will be null but an empty object.

skipping/removing the key for empty design makes it difficult to understand whether design is actually empty or have got corrupted.

@leecalcote
Copy link
Member

The services key as you said should be ever present, and this PR ensures this happens. Comparing it with primary constraint not null, if design is empty, it is not that services will be null but an empty object.

Aha! Ok, that’s very good.

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

Successfully merging this pull request may close these issues.

None yet

2 participants