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

Fix merging of source status objects #1109

Merged
merged 3 commits into from
Oct 22, 2020

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Oct 22, 2020

📔 Description

Unlike for other merge operations, for the status we want to recurse deep into the object structure and merge array contents instead of overwriting arrays. This fixes an issue that caused only the sources' status to show up in the output of vast status --detailed that replied last.

I verified this to work locally. We don't have an integration test that covers this.

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

Please run this locally for testing. We do not have an integration test that covers this.

@dominiklohmann dominiklohmann added the bug Incorrect behavior label Oct 22, 2020
@dominiklohmann dominiklohmann requested a review from a team October 22, 2020 08:49
@dominiklohmann dominiklohmann force-pushed the story/ch19323/deep-merge-status branch 3 times, most recently from 7ee4aae to cf42956 Compare October 22, 2020 09:22
libvast/vast/detail/settings.hpp Outdated Show resolved Hide resolved
Unlike for other merge operations, for the status we want to recurse
deep into the object structure and merge array contents instead of
overwriting arrays.  This fixes an issue that caused only the sources'
status to show up in the output of `vast status` that replied last.
@dominiklohmann dominiklohmann merged commit f135043 into master Oct 22, 2020
@dominiklohmann dominiklohmann deleted the story/ch19323/deep-merge-status branch October 22, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants