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

Sort loaded plugins by name #1679

Merged
merged 1 commit into from
May 25, 2021
Merged

Conversation

dominiklohmann
Copy link
Member

📔 Description

This ensures the version comparison is agnostic to the order of the plugins.

📝 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

Connect a local VAST to a VAST running the same plugins specified in a different order and notice VAST no longer warns unnecessarily.

@dominiklohmann dominiklohmann added the bug Incorrect behavior label May 25, 2021
@dominiklohmann dominiklohmann requested a review from a team May 25, 2021 13:14
This ensures the version comparison is agnostic to the order of the
plugins.
@dominiklohmann dominiklohmann force-pushed the story/ch25770/plugin-load-order branch from fc0d4af to 78438f7 Compare May 25, 2021 13:16
Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

I don't have a setup to test, but looking it the code it seems pretty safe to say that it should work.

Relying on the order being setup during startup is a very implicit way of doing this, but long-term the issue should resolve itself anyways as soon as record comparisons become order-independent.

@dominiklohmann
Copy link
Member Author

I don't have a setup to test, but looking it the code it seems pretty safe to say that it should work.

A better fix would probably be to use a stable set for this with a custom equal_to function object, but that is far more than we need to fix this before the release. I'll write a story for that.

@dominiklohmann dominiklohmann merged commit a260850 into master May 25, 2021
@dominiklohmann dominiklohmann deleted the story/ch25770/plugin-load-order branch May 25, 2021 14:00
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