Stefan Schubert write:
> ref: refs/heads/master
> commit 31f399a35839d17d62005b9623562d53af7a6092
> Author: Stefan Schubert <[email protected]>
> 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: [email protected]
For additional commands, e-mail: [email protected]
