Add more server actions #149
Conversation
None of these actions have complicated request arguments or have any responses returned.
These are actions which, while having return values, are relatively simple in what they take as arguments.
|
The functional tests are filing on this. |
Some server actions return an empty body, which is not valid JSON.
|
The test |
Yeah, I've noticed it too. I suspect retrying for a longer time is the right way. |
|
@milliams hi, are you still interested in this? |
milliams
force-pushed
the
add_server_actions
branch
from
7431ad6
to
fc6d4f2
Compare
|
@milliams yep, just merged it. |
milliams
force-pushed
the
add_server_actions
branch
2 times, most recently
from
e40f060
to
e644b15
Compare
| Ok(ServerStatusWaiter { | ||
| server: self, | ||
| target: protocol::ServerStatus::ShutOff, | ||
| }) | ||
| } | ||
|
|
||
| /// Run an action on the server. | ||
| pub async fn action(&mut self, action: ServerAction) -> Result<()> { | ||
| pub async fn action(&mut self, action: ServerAction) -> Result<Option<serde_json::Value>> { |
There was a problem hiding this comment.
I wonder if we can consider this a breaking change (because Rust will insist on let _ = ... by default). It's not an objection, just wondering how I should release rust-openstack after this.
There was a problem hiding this comment.
I'm also really unsure about using serde_json::Value in a public function. I'm trying to design the high-level API here to be type safe. Is it only for GetConsoleOutput? Maybe make separate methods for actions that return something?
| @@ -343,19 +343,125 @@ impl Server { | |||
| #[non_exhaustive] | |||
| #[allow(missing_copy_implementations)] | |||
There was a problem hiding this comment.
nit: this can be dropped now, Copy is no longer possible
| Restore, | ||
| /// Shows console output for a server. | ||
| #[serde(rename = "os-getConsoleOutput")] | ||
| OsGetConsoleOutput { |
There was a problem hiding this comment.
Could you remove the "Os" prefix? I don't think it's meaningful, is it?
|
@milliams hey, are you still interested in this PR? If you no longer have time, I can merge it and clean up myself. |
|
I'm not sure that I'll have time to make the changes on this, so feel free to hack it as you see fit. |
This adds most of the remaining server actions.
This includes all of the simple actions which have very single or no arguments taken and return nothing, as well as some which take relatively simple arguments (e.g. a few strings) but might return values.
The first question is, given the diversity of return values given by these actions, is it ok for us to provide that as a generic
serde_json::Value, or should we define deserialisable structs for each action that provides a value? Perhaps the genericServer::actionshould return aserde_json::Value, but if we implement specific functions for any other actions in the future (like we already have for e.g.Server::stop) we define return type structs/types for those?I did a quick survey and at the moment, there are only 6 actions which return anything and if we want could probably be encoded as:
So probably in that situation, the wrapper functions would be simple enough.
Secondly, there are some more complex actions not implemented here. Is that ok that they will be added in some later version of rust-openstack? I assume so, given the
#[non_exhaustive]. I expect that OpenStack will add more in the future too, so that covers that also.