Untangled Go RPC code #1291
Untangled Go RPC code #1291
Conversation
| @@ -0,0 +1,308 @@ | |||
| /* ** DEPRECATED ** | |||
There was a problem hiding this comment.
We probably want this separate from the copyright notice separated by a line in it's own comment block. We want attention to be drawn to it.
| @@ -107,6 +107,9 @@ func newAvailablePlugin(resp plugin.Response, emitter gomit.Emitter, ep executab | |||
| } | |||
| ap.client = c | |||
| case plugin.NativeRPC: | |||
| log.WithFields(log.Fields{ | |||
| "_module": "control-aplugin", | |||
| }).Warning("This plugin has been deprecated.") | |||
There was a problem hiding this comment.
We need more description here.
There was a problem hiding this comment.
The suggested changes have been made
| @@ -107,6 +107,9 @@ func newAvailablePlugin(resp plugin.Response, emitter gomit.Emitter, ep executab | |||
| } | |||
| ap.client = c | |||
| case plugin.NativeRPC: | |||
| log.WithFields(log.Fields{ | |||
There was a problem hiding this comment.
Give me the block also (newAvailablePlugin) so that I know exactly where this message came from.
kjlyon
force-pushed
the
ib/Deprecate-legacy-plugin-lib
branch
2 times, most recently
from
3e4884b to
cceb3f4
Compare
| @@ -1,3 +1,5 @@ | |||
| /* ** DEPRECATED ** */ | |||
There was a problem hiding this comment.
The deprecated banner should also include a link to #1289.
There was a problem hiding this comment.
I made the suggested updates
| // These are our built-in content types for plugins | ||
| // SnapProtoBuff snap metrics serialized into protocol buffers | ||
| SnapProtoBuff = "snap.pb" // TO BE IMPLEMENTED | ||
| ) |
There was a problem hiding this comment.
This constant isn't used and could be removed.
| "net/rpc/jsonrpc" | ||
| "regexp" | ||
| "runtime" | ||
| "crypto/rsa" // Don't use "fmt.Print*" |
There was a problem hiding this comment.
This comment // Don't use "fmt.Print*" should be moved up below the warning or removed.
kjlyon
force-pushed
the
ib/Deprecate-legacy-plugin-lib
branch
from
cceb3f4 to
c22d877
Compare
| return &common.Empty{}, err | ||
| } | ||
| return &common.Empty{}, nil | ||
| }*/ |
There was a problem hiding this comment.
Do we want to leave this stuff in here commented out?
There was a problem hiding this comment.
I left it there since it was in publisher_proxy before I untangled the code. I don't know if it has a purpose or future purpose.
There was a problem hiding this comment.
It doesn't have a future purpose and should've been removed previously.
There was a problem hiding this comment.
Makes sense. I deleted that section and a couple other places that were just commented out code.
kjlyon
force-pushed
the
ib/Deprecate-legacy-plugin-lib
branch
from
c22d877 to
81caf55
Compare
| "_module": "control-aplugin", | ||
| "_block": "newAvailablePlugin", | ||
| "aplugin": ap.name, | ||
| }).Warning("This plugin is using a deprecated RPC protocol. Please update " + ap.name) |
There was a problem hiding this comment.
I think this would look better if we change aplugin to plugin_name and then remove the name from the warning string. Instead it could be something like This plugin is using a deprecated RPC protocol.. I'm not convinced if we should have some sort of message pointing them somewhere (a url?) where they can find out more. What do you think?
- Identified code used by GoRPC plugins - Created files with deprecated flags to hold code that is used by GoRPC plugins - Added deprecation warning when loading legacy plugin
kjlyon
force-pushed
the
ib/Deprecate-legacy-plugin-lib
branch
from
81caf55 to
0a30d04
Compare
Enables #1289 to be done easily when the time comes
Summary of changes:
Testing done:
@intelsdi-x/snap-maintainers