Open
Description
While figuring out acceptance tests for vagrant-google, I found a weird logic piece in the implementation of synchronous operations in Fog.
As an example, let's take a look at Server class' destroy method:
def destroy(async=true)
requires :name, :zone
data = service.delete_server(name, zone_name)
operation = Fog::Compute::Google::Operations.new(:service => service).get(data.body['name'], data.body['zone'])
unless async
operation.wait_for { ready? }
end
operation
end
The async parameter is just a true/false switch, so if we need to perform the operation synchronously (important for tests for example), we need to specify it like so:
instance.destroy(false)
Which, I find highly confusing to understand for someone who's reading the code later.
Due to the default being true, it is not easy to wrap around with a statement, since it will not make any sense either:
async_execution = false
instance.destroy(async_execution)
I was wandering - maybe it makes sense to make it a named parameter?
This will allow for:
- Logically sound statements:
instance.start(async: false)
- Ability to write in more execution flow control parameters easily if we need them.