Skip to content

Centralize Label Prefiltering and Matching#299

Open
VJanKraemer wants to merge 7 commits intomainfrom
new_labelmatcher
Open

Centralize Label Prefiltering and Matching#299
VJanKraemer wants to merge 7 commits intomainfrom
new_labelmatcher

Conversation

@VJanKraemer
Copy link
Contributor

Fix the bug found in #294

Centralize the Prefiltering and Matching

Developer checklist (address before review)

  • Changelog.md updated
  • Prepared update for depending repositories
  • Documentation updated (public API changes only)
  • API docstrings updated (public API changes only)
  • Rebase → commit history clean
  • Squash and merge → proper PR title

* Adds a getValue function for the specified key with proper error
  handling included

Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
@VJanKraemer VJanKraemer added the bug Something isn't working label Mar 9, 2026
VJanKraemer and others added 2 commits March 9, 2026 10:14
In SpecificDiscoveryStore, add the label set along with the handler info
to the DiscoveryCluster as "Controller" info. This allows us to do the
label matching centralized in the DiscoveryStore

Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
* Add test to test whether the optional label prefiltering works when
  the publishers are started before the Subscriber

Signed-off-by: Konrad Breitsprecher <Konrad.Breitsprecher@vector.com>
Co-authored-by: Jan Kraemer <jan.kraemer@vector.com>

#include "LabelMatching.hpp"
#include <optional>
#include <iostream>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#include <iostream>

Copy link
Member

Choose a reason for hiding this comment

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

The name noLabelTestDescriptor in this test is wrong, no? The baseDescriptor has a single label set (mandatory, key kA, value vA). I suppose this was copy-pasted way-back-then from the test above, where the baseDescriptor actually has no labels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what you get when you cherry pick commits without properly reviewing them :D No I stumbled across this as well but missed to fix it. Thanks :)

MariusBgm and others added 3 commits March 9, 2026 16:26
Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
Co-authored-by: Jan Kraemer <jan.kraemer@vector.com>
* Add comments to better explain the used algorithm, since it is
  somewhat convoluted

Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants