This repository has been archived by the owner on Nov 8, 2022. It is now read-only.
Client libs to enable non-go language plugins #1150
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updates control package to use metrics throughout the workflow. This new function prototype had to be changed in a few places. Removes josnrc process/publish client creation since it is only able to be used for collection. Adds invalid rpctype error if unable to match the incoming rpctype.
Updates plugin package. Removes grpc server actions from the package since this is being moved to a new plugin-lib repo. Updates client package to use new function prototypes of passing metric all the way through a workflow instead of bytes through process/publish. For native/gob, the encoding/decoding now happens in the client. This is done because the concept of contentType was removed form the framework but we still want to support existing plugins. Doing this in the client allows existing plugins to interact with plugins written using any other langauge lib. Updates proto definitions, including adding some types that were previously referenced from common. This was done to allow plugin-libs written in other languages to only require one proto file to generate from, reducing complexity for the plugin-lib authors at the cost of some duplication.
In cpolicy, when Merge is called it modifies the callers rules which is not the desired behavior. This adds the creation of a new node that is used to contain the merged rules.
Updates controlproxy to match new managesMetrics interface. Updates tests and proto files to match.
Updates scheduler managesMetrics interface to use core.Metric all the way through a workflow instead of switching to bytes for process and publish. This removes the un-implemented contentType feature and associated testing/partial implementations.
| type collectsMetrics interface { | ||
| CollectMetrics(string, map[string]map[string]string) ([]core.Metric, []error) | ||
| } | ||
|
|
||
| type publishesMetrics interface { | ||
| PublishMetrics(contentType string, content []byte, pluginName string, pluginVersion int, config map[string]ctypes.ConfigValue, taskID string) []error | ||
| PublishMetrics([]core.Metric, map[string]ctypes.ConfigValue, string, string, int) []error | ||
| //PublishMetrics(contentType string, content []byte, pluginName string, pluginVersion int, config map[string]ctypes.ConfigValue, taskID string) []error | ||
| } |
There was a problem hiding this comment.
Remove the comment here?
|
When this lands, we can bring #660 back up 👌 |
This was referenced Aug 18, 2016
| br, err = cpolicy.NewBoolRule(key, val.Required, val.Default) | ||
| } else { | ||
| br, err = cpolicy.NewBoolRule(key, val.Required) | ||
| } | ||
| if err != nil { | ||
| // The only error that can be thrown is empty key error, ignore something with empty key |
There was a problem hiding this comment.
If we call the plugin's GetConfigPolicy and receive a ConfigPolicyNode which contains an empty key we might want to log this (level=warning). What do you think?
There was a problem hiding this comment.
I added warning statement here. I don't have a strong opinion on it.
|
LGTM |
|
LGTM |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially addresses #965
Via some changes re: content-types, resolves #771.
Summary of changes:
These changes are geared around enabling non-go plugins to be written by using grpc. It moves some of the go specific code to the clients (where we differentiate between the native/go client and grpc client) so that native clients and grpc clients can co-exist in the same workflow (and outside of the clients, they follow the same code path through snapd).
For a more specific summary see the commit messages.
Things to note:
Things not included in this PR:
Testing done:
@intelsdi-x/snap-maintainers