Mailinglist Archive: yast-devel (77 mails)

< Previous Next >
[yast-devel] Re: [yast-commit] <rest-service> master : code cleanup
  • From: Josef Reidinger <jreidinger@xxxxxxx>
  • Date: Tue, 22 Dec 2009 12:45:32 +0100
  • Message-id: <200912221245.32372.jreidinger@xxxxxxx>
Stefan Schubert write:
ref: refs/heads/master
commit 31f399a35839d17d62005b9623562d53af7a6092
Author: Stefan Schubert <schubi@xxxxxxx>
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@xxxxxxxxxxxx
For additional commands, e-mail: yast-devel+help@xxxxxxxxxxxx

< Previous Next >
This Thread
  • No further messages