search

fog / fog-xenserver

Module for the 'fog' gem to support XENSERVER
MIT License
16 stars 22 forks source link

Support master and slave #76

Closed zozoh94 closed 5 years ago

zozoh94 commented 5 years ago

I made a PR few month ago and you merged it. But some new tests recently reveals that the process of finding master URL from slave doesn't work as I imagine. This is a new PR which works well.

zozoh94 commented 5 years ago

I created a new branch from master because the rebase was not simple with file renaming.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 94.697% when pulling 53a6f676b136fb6dc4b6d6ac13d31754fd235905 on zozoh94:redirect_to_master into b6752f916dc73dd1ce04e8d243469164f15a2cca on fog:master.

plribeiro3000 commented 5 years ago

Hey @zozoh94. Thanks!

Whats the issue with the current approach where the lib does it automatically without the need of a configuration parameter?

zozoh94 commented 5 years ago

The problem is in lib/fog/xenserver/compute/real.rb line 19 because it will raise an Exception when xenurl is slave. Inside connection.rb response["status"] will not be equal to "Success".

plribeiro3000 commented 5 years ago

Can't we rescue the exception and keep it they way it is?

My concern is that since you can't do anything on a slave it does not make sense to have a parameter to force this. It should always connect to the master node automatically.

zozoh94 commented 5 years ago

You're right ! Is it better like that ?

plribeiro3000 commented 5 years ago

@zozoh94 LGTM except for some small details. I will happily bring this in once you fix them. 😄

plribeiro3000 commented 5 years ago

Thanks for your efforts here @zozoh94