From: Martyn Taylor mtaylor@redhat.com
--- src/app/util/condormatic.rb | 2 ++ src/config/condor.yml | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) create mode 100644 src/config/condor.yml
diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb index b52ff46..45aa4bd 100644 --- a/src/app/util/condormatic.rb +++ b/src/app/util/condormatic.rb @@ -29,6 +29,7 @@ class Possible @hwp = hwp @provider_image = provider_image @realm = realm + @condor_config = YAML.load_file("#{RAILS_ROOT}/config/condor.yml") end end
@@ -93,6 +94,7 @@ def condormatic_instance_create(task) pipe_and_log(pipe, "DeltacloudUsername = #{found.account.credentials_hash['username']}\n") pipe_and_log(pipe, "DeltacloudPasswordFile = #{pwfilename}") pipe_and_log(pipe, "DeltacloudImageId = #{found.provider_image.target_identifier}\n") + pipe_and_log(pipe, "DeltacloudRetryTimeout = #{@condor_config[:request_timeout]}\n") pipe_and_log(pipe, "DeltacloudHardwareProfile = #{found.hwp.external_key}\n") pipe_and_log(pipe, diff --git a/src/config/condor.yml b/src/config/condor.yml new file mode 100644 index 0000000..9db1e5b --- /dev/null +++ b/src/config/condor.yml @@ -0,0 +1 @@ +:request_timeout: 180
On 07/27/11 - 04:56:14PM, mtaylor@redhat.com wrote:
From: Martyn Taylor mtaylor@redhat.com
src/app/util/condormatic.rb | 2 ++ src/config/condor.yml | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) create mode 100644 src/config/condor.yml
diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb index b52ff46..45aa4bd 100644 --- a/src/app/util/condormatic.rb +++ b/src/app/util/condormatic.rb @@ -29,6 +29,7 @@ class Possible @hwp = hwp @provider_image = provider_image @realm = realm
- @condor_config = YAML.load_file("#{RAILS_ROOT}/config/condor.yml") end
end
@@ -93,6 +94,7 @@ def condormatic_instance_create(task) pipe_and_log(pipe, "DeltacloudUsername = #{found.account.credentials_hash['username']}\n") pipe_and_log(pipe, "DeltacloudPasswordFile = #{pwfilename}") pipe_and_log(pipe, "DeltacloudImageId = #{found.provider_image.target_identifier}\n")
- pipe_and_log(pipe, "DeltacloudRetryTimeout = #{@condor_config[:request_timeout]}\n") pipe_and_log(pipe, "DeltacloudHardwareProfile = #{found.hwp.external_key}\n") pipe_and_log(pipe,
Hm, I'm not sure I love these kinds of patches. In particular, I want us to start getting away from using configuration files, because the administrator has no idea on how to set them and they are not dynamic enough. For instance, in this case, what value would you want the administrator to set? In what circumstances would he change it? If you add a new cloud provider type, is the timeout correct?
In this case, I think we can do a lot better by modifying the timeout ourselves. That is, by this point of the code, we know which provider backend we are going to be using. So we should use that information and do a table lookup.
EC2? -> default timeout VMware -> 180 seconds RHEV -> 90 seconds etc.
(note that I just made the above numbers up, they can be whatever we need to be correct)
On 07/28/2011 09:19 AM, Chris Lalancette wrote:
On 07/27/11 - 04:56:14PM, mtaylor@redhat.com wrote:
From: Martyn Taylor mtaylor@redhat.com
src/app/util/condormatic.rb | 2 ++ src/config/condor.yml | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) create mode 100644 src/config/condor.yml
diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb index b52ff46..45aa4bd 100644 --- a/src/app/util/condormatic.rb +++ b/src/app/util/condormatic.rb @@ -29,6 +29,7 @@ class Possible @hwp = hwp @provider_image = provider_image @realm = realm
- @condor_config = YAML.load_file("#{RAILS_ROOT}/config/condor.yml") end
end
@@ -93,6 +94,7 @@ def condormatic_instance_create(task) pipe_and_log(pipe, "DeltacloudUsername = #{found.account.credentials_hash['username']}\n") pipe_and_log(pipe, "DeltacloudPasswordFile = #{pwfilename}") pipe_and_log(pipe, "DeltacloudImageId = #{found.provider_image.target_identifier}\n")
- pipe_and_log(pipe, "DeltacloudRetryTimeout = #{@condor_config[:request_timeout]}\n") pipe_and_log(pipe, "DeltacloudHardwareProfile = #{found.hwp.external_key}\n") pipe_and_log(pipe,
Hm, I'm not sure I love these kinds of patches. In particular, I want us to start getting away from using configuration files, because the administrator has no idea on how to set them and they are not dynamic enough. For instance, in this case, what value would you want the administrator to set? In what circumstances would he change it? If you add a new cloud provider type, is the timeout correct?
In this case, I think we can do a lot better by modifying the timeout ourselves. That is, by this point of the code, we know which provider backend we are going to be using. So we should use that information and do a table lookup.
EC2? -> default timeout VMware -> 180 seconds RHEV -> 90 seconds etc.
(note that I just made the above numbers up, they can be whatever we need to be correct)
Hi Chris,
I think you have a fair point. Although I was specifically tasked with making this value configurable.
How about I put a table like you described above in the config file? This allows us to gives the best of both worlds.
I understand what you are saying with regards to configuration being complex, though I think an administrator would/should have enough information to make a decision on these types of things. Maybe that means we need documentation to go along side the configuration. Say my VMWare server, is really Slow, I might need to tinker with some of the values.
I do think we could do with an amalgamation of the many files we have under config folder and we need to document the resulting file appropriately.
On 07/28/11 - 10:11:58AM, Martyn Taylor wrote:
On 07/28/2011 09:19 AM, Chris Lalancette wrote:
On 07/27/11 - 04:56:14PM, mtaylor@redhat.com wrote:
From: Martyn Taylor mtaylor@redhat.com
src/app/util/condormatic.rb | 2 ++ src/config/condor.yml | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) create mode 100644 src/config/condor.yml
diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb index b52ff46..45aa4bd 100644 --- a/src/app/util/condormatic.rb +++ b/src/app/util/condormatic.rb @@ -29,6 +29,7 @@ class Possible @hwp = hwp @provider_image = provider_image @realm = realm
- @condor_config = YAML.load_file("#{RAILS_ROOT}/config/condor.yml") end
end
@@ -93,6 +94,7 @@ def condormatic_instance_create(task) pipe_and_log(pipe, "DeltacloudUsername = #{found.account.credentials_hash['username']}\n") pipe_and_log(pipe, "DeltacloudPasswordFile = #{pwfilename}") pipe_and_log(pipe, "DeltacloudImageId = #{found.provider_image.target_identifier}\n")
- pipe_and_log(pipe, "DeltacloudRetryTimeout = #{@condor_config[:request_timeout]}\n") pipe_and_log(pipe, "DeltacloudHardwareProfile = #{found.hwp.external_key}\n") pipe_and_log(pipe,
Hm, I'm not sure I love these kinds of patches. In particular, I want us to start getting away from using configuration files, because the administrator has no idea on how to set them and they are not dynamic enough. For instance, in this case, what value would you want the administrator to set? In what circumstances would he change it? If you add a new cloud provider type, is the timeout correct?
In this case, I think we can do a lot better by modifying the timeout ourselves. That is, by this point of the code, we know which provider backend we are going to be using. So we should use that information and do a table lookup.
EC2? -> default timeout VMware -> 180 seconds RHEV -> 90 seconds etc.
(note that I just made the above numbers up, they can be whatever we need to be correct)
Hi Chris,
I think you have a fair point. Although I was specifically tasked with making this value configurable.
How about I put a table like you described above in the config file? This allows us to gives the best of both worlds.
I understand what you are saying with regards to configuration being complex, though I think an administrator would/should have enough information to make a decision on these types of things. Maybe that means we need documentation to go along side the configuration. Say my VMWare server, is really Slow, I might need to tinker with some of the values.
I guess that could be the case. I like your idea of doing the lookup table in the config file; that is, we provide reasonable default values for the various, and the administrator can tinker if needed.
I do think we could do with an amalgamation of the many files we have under config folder and we need to document the resulting file appropriately.
Well, there are 2 things here: 1) We should actually think about consolidating all of the configuration into a "config.ini" file. That will just make it easier to find things. 2) We should really evaluate if these configuration type things should be settable from the UI. It might make for a nicer user experience that way.
On 07/28/2011 11:11 AM, Chris Lalancette wrote:
On 07/28/11 - 10:11:58AM, Martyn Taylor wrote:
On 07/28/2011 09:19 AM, Chris Lalancette wrote:
On 07/27/11 - 04:56:14PM, mtaylor@redhat.com wrote:
From: Martyn Taylor mtaylor@redhat.com
src/app/util/condormatic.rb | 2 ++ src/config/condor.yml | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) create mode 100644 src/config/condor.yml
diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb index b52ff46..45aa4bd 100644 --- a/src/app/util/condormatic.rb +++ b/src/app/util/condormatic.rb @@ -29,6 +29,7 @@ class Possible @hwp = hwp @provider_image = provider_image @realm = realm
- @condor_config = YAML.load_file("#{RAILS_ROOT}/config/condor.yml") end
end
@@ -93,6 +94,7 @@ def condormatic_instance_create(task) pipe_and_log(pipe, "DeltacloudUsername = #{found.account.credentials_hash['username']}\n") pipe_and_log(pipe, "DeltacloudPasswordFile = #{pwfilename}") pipe_and_log(pipe, "DeltacloudImageId = #{found.provider_image.target_identifier}\n")
- pipe_and_log(pipe, "DeltacloudRetryTimeout = #{@condor_config[:request_timeout]}\n") pipe_and_log(pipe, "DeltacloudHardwareProfile = #{found.hwp.external_key}\n") pipe_and_log(pipe,
Hm, I'm not sure I love these kinds of patches. In particular, I want us to start getting away from using configuration files, because the administrator has no idea on how to set them and they are not dynamic enough. For instance, in this case, what value would you want the administrator to set? In what circumstances would he change it? If you add a new cloud provider type, is the timeout correct?
In this case, I think we can do a lot better by modifying the timeout ourselves. That is, by this point of the code, we know which provider backend we are going to be using. So we should use that information and do a table lookup.
EC2? -> default timeout VMware -> 180 seconds RHEV -> 90 seconds etc.
(note that I just made the above numbers up, they can be whatever we need to be correct)
Hi Chris,
I think you have a fair point. Although I was specifically tasked with making this value configurable.
How about I put a table like you described above in the config file? This allows us to gives the best of both worlds.
I understand what you are saying with regards to configuration being complex, though I think an administrator would/should have enough information to make a decision on these types of things. Maybe that means we need documentation to go along side the configuration. Say my VMWare server, is really Slow, I might need to tinker with some of the values.
I guess that could be the case. I like your idea of doing the lookup table in the config file; that is, we provide reasonable default values for the various, and the administrator can tinker if needed.
Great, I will amend the patch accordingly.
I do think we could do with an amalgamation of the many files we have under config folder and we need to document the resulting file appropriately.
Well, there are 2 things here:
- We should actually think about consolidating all of the configuration into
a "config.ini" file. That will just make it easier to find things. 2) We should really evaluate if these configuration type things should be settable from the UI. It might make for a nicer user experience that way.
Yeah I definitely agree wrt UI, We did start some work on settings in UI a while back, but as plans changed I think it was dropped.
Thanks for your comments :)
Martyn
aeolus-devel@lists.fedorahosted.org
-
Chris Lalancette -
Martyn Taylor