uploader: add PluginControl handshake protos

[wchargin]

Summary:
This enables us to flag features on the server side, so that we can
stage releases, and so that we can roll back in a pinch without
requiring an expedited PyPI push and end user version upgrade.

Test Plan:
No new behavior in this change. The protos still build.

wchargin-branch: uploader-plugincontrol-protos

[davidsoergel]

Maybe reword this comment a bit: which data to send is still at the discretion of the client, within the constraints given by this field.

[wchargin]

Good point. Done.

[davidsoergel]

I like how generic this message is. We could in principle extend it with rate limits per plugin, informational messages per plugin, whatever makes sense later.

A slightly different design might prepare us for that eventuality even better. Instead of one PluginControl for all plugins, you could do repeated PluginControl plugin_controls; where for now PluginControl contains just string plugin_name. Or do that in a nested manner with AllPluginControls where you have it, with repeated SinglePluginControl inside (mod naming).

(Just to consider; I'm OK with it either way)

[wchargin]

I like how generic this message is. We could in principle extend it
with rate limits per plugin, informational messages per plugin,
whatever makes sense later.

Yep, exactly. Rate limits were something that I had in mind, as was a
flag “bool allow_all_plugins = 2;” for general-purpose storage (e.g.,
if people run their own instances, they might want that).

A slightly different design might prepare us for that eventuality even
better […]

A downside of repeated PluginControl plugin_controls; is that it
doesn’t admit distinguishing between an old server that doesn’t know
about PluginControls and a server that is denying all uploads. If we
did that, then rolling back the server to a pre-PluginControl version
would be dangerous once we’d pushed a new uploader, so I like having the
wrapper message.

An AllPluginControls with repeated SinglePluginControls does resolve
this, and indeed is more flexible along the plugin axis. But it’s a bit
less flexible on other axes: e.g., I could imagine wanting

message ServerInfoResponse {
  // ...
  PluginControl plugin_control = 4;
  repeated RateLimit rate_limit = 5;
}

message RateLimit {
  oneof what {
    // Rate-limit a specific plugin.
    string plugin_name = 1;
    // Rate-limit all data of a given data class.
    .tensorboard.DataClass data_class = 2;
  }
  // Send requests at most once every this often.
  Duration min_request_period = 3;
}

in which case the SinglePluginControls aren’t obviously the right
place to put the rate limiting.

Given that both options seem reasonable and the worst-case costs aren’t
too high (we can either deprecate the old fields entirely, or use a
hybrid solution like denormalizing plugin names or using the plugin
control ordering as an index; neither is perfect, but they’re not
maintenance nightmares), I’m going to keep this as is, because this way
is simpler right now. Thank you for the suggestion!

[wchargin]

Thanks for the review!

[wchargin]

Good point. Done.

[wchargin]

I like how generic this message is. We could in principle extend it
with rate limits per plugin, informational messages per plugin,
whatever makes sense later.

Yep, exactly. Rate limits were something that I had in mind, as was a
flag “bool allow_all_plugins = 2;” for general-purpose storage (e.g.,
if people run their own instances, they might want that).

A slightly different design might prepare us for that eventuality even
better […]

A downside of repeated PluginControl plugin_controls; is that it
doesn’t admit distinguishing between an old server that doesn’t know
about PluginControls and a server that is denying all uploads. If we
did that, then rolling back the server to a pre-PluginControl version
would be dangerous once we’d pushed a new uploader, so I like having the
wrapper message.

An AllPluginControls with repeated SinglePluginControls does resolve
this, and indeed is more flexible along the plugin axis. But it’s a bit
less flexible on other axes: e.g., I could imagine wanting

message ServerInfoResponse {
  // ...
  PluginControl plugin_control = 4;
  repeated RateLimit rate_limit = 5;
}

message RateLimit {
  oneof what {
    // Rate-limit a specific plugin.
    string plugin_name = 1;
    // Rate-limit all data of a given data class.
    .tensorboard.DataClass data_class = 2;
  }
  // Send requests at most once every this often.
  Duration min_request_period = 3;
}

in which case the SinglePluginControls aren’t obviously the right
place to put the rate limiting.

Given that both options seem reasonable and the worst-case costs aren’t
too high (we can either deprecate the old fields entirely, or use a
hybrid solution like denormalizing plugin names or using the plugin
control ordering as an index; neither is perfect, but they’re not
maintenance nightmares), I’m going to keep this as is, because this way
is simpler right now. Thank you for the suggestion!