Issue 29, check for bad client version #40
Conversation
| @@ -13,6 +13,17 @@ using namespace std::placeholders; | |||
| namespace dist_clang { | |||
| namespace daemon { | |||
|
|
|||
| namespace { | |||
|
|
|||
| bool ValidateClientVersion(ui32 client_version) { | |||
There was a problem hiding this comment.
Looking at this function only I can say that this approach is not anticipated. The backward-compatible version should be the field in the Absorber config. Since the usual situation is when the clients (Emitters) are updated much more often than Absorbers, it's convenient to update this field without redistributing the new package.
There was a problem hiding this comment.
Good point, will do.
dreamer-dead
force-pushed
the
issue-29
branch
2 times, most recently
from
89dee14 to
4b056fd
Compare
dreamer-dead
force-pushed
the
issue-29
branch
from
4b056fd to
09cb638
Compare
|
Added a new field in Absorber config with tests. |
| @@ -7,6 +7,7 @@ package dist_clang.daemon.proto; | |||
| message Remote { | |||
| optional base.proto.Flags flags = 1; | |||
| optional string source = 2; | |||
| optional uint32 emitter_version = 3 [ default = 100 ]; | |||
There was a problem hiding this comment.
Let it be simply "version" - and the default value should be real, i.e. the current version (790.0). Also the real version is complex and consists of two numbers: better to reflect it as a separate message type, like:
message Version {
optional uint32 major = 1 [ default = 790 ];
optional uint32 minor = 2 [ default = 0];
}
This message type should be shared with a its counterpart in Absorber message.
|
Done with review. |
Hi!
Here is my initial version of client version validation (#29), so we can discuss this approach.
Constant 100 for version is a sample only.
Should we expose that minimal version value to C++ code and keep it sync with the default value in proto file?
Please, take a look.