Stefan Schubert write:

ref: refs/heads/master commit 31f399a35839d17d62005b9623562d53af7a6092 Author: Stefan Schubert <schubi@suse.de> Date: Tue Dec 22 12:02:12 2009 +0100

code cleanup --- .../status/app/controllers/metrics_controller.rb | 36 ++++++------------- 1 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/plugins/status/app/controllers/metrics_controller.rb b/plugins/status/app/controllers/metrics_controller.rb index d39205f..719b6ee 100644 --- a/plugins/status/app/controllers/metrics_controller.rb +++ b/plugins/status/app/controllers/metrics_controller.rb @@ -58,20 +58,10 @@ class MetricsController < ApplicationController end limits = Hash.new limits = create_limit(params["status"]) - f = File.open(File.join(plugin_config_dir, "status_limits.yaml"), "w") + f = File.open(File.join(plugin_config_dir, "status_configuration.yaml"), "w") f.write(limits.to_yaml) f.close - - @status = Status.new - # use now if time is not valid - begin - stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i) - start = params[:start].blank? ? stop - 300 : Time.at(params[:start].to_i) - @status.collect_data(start, stop, params[:data]) - render :show - rescue Exception => e - render :text => e.to_s, :status => 400 # bad request - end + render :text => "OK"

^^^ I think that it is not good idea to render text, as you maybe want to check it on frontend and ActiveResource show have problem with it.

end

# GET /status @@ -88,12 +78,13 @@ class MetricsController < ApplicationController end end

- @metrics = Metric.find(:all, conditions) - - respond_to do |format| - format.xml { render :xml => @metrics.to_xml(:root => 'metrics', :data => false) } - end + @metric = Metric.find(:all, conditions)

+ @data = false + @start = nil + @stop = nil + + render :show

^^^^ I think you forget to add show template to render I see only status one.

end

# GET /status/1 @@ -102,14 +93,11 @@ class MetricsController < ApplicationController #permission_check("org.opensuse.yast.system.status.read") begin @metric = Metric.find(params[:id]) - stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i) - start = params[:start].blank? ? stop - 300 : Time.at(params[:start].to_i) - - respond_to do |format| - format.xml { render :xml => @metric.to_xml(:start => start, :stop => stop) } - end + @stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i) + @start = params[:start].blank? ? @stop - 300 : Time.at(params[:start].to_i) ^^^^ 300 should be defined constant such as DEFAULT_RANGE=300 or similar which say context of number. + @data = true rescue Exception => e - render :text => e.to_s, :status => 400 # bad request + render ErrorResult.error(400, 108, e.to_s) and return ^^^ I think it should use same way for reporting exception as rest of webservice, then you don't need to handle client exception (and just server) on frontend. And if I understand code, you want to report bad request, which is typical due to bad parameters and for this purpose you should use InvalidParameters exception which correctly report it to ActiveResource, so you can easily check result of save method on frontend (it behaves as common rails application). end end end

-- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, webyast (language,time,basesystem,ntp) -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org