September 2009 - YaST Development - openSUSE Mailing Lists

[yast-devel] Re: [yast-commit] <web-client> basesystem : when module failed in control panel, the result was endless redirect loop
by Josef Reidinger 03 Sep '09

03 Sep '09

On 08/26/2009 06:58 PM, Martin Kudlvasr wrote: > ref: refs/heads/basesystem > commit ea5740fcf91f51284a51cb1bf9345823ddd57cee > Author: Martin Kudlvasr <mkudlvasr(a)suse.cz> > Date: Wed Aug 26 18:58:19 2009 +0200 > > when module failed in control panel, the result was endless redirect loop > added another view to separate controlpanel from basesystem. > now user actually see the error > --- > .../app/controllers/controlpanel_controller.rb | 25 +++++++++++++------ > webclient/app/views/controlpanel/basesystem.rhtml | 4 +++ > 2 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/webclient/app/controllers/controlpanel_controller.rb b/webclient/app/controllers/controlpanel_controller.rb > index b9ba795..09d7494 100644 > --- a/webclient/app/controllers/controlpanel_controller.rb > +++ b/webclient/app/controllers/controlpanel_controller.rb > @@ -38,8 +38,6 @@ class ControlpanelController < ApplicationController > > > # Constant that signalizes, that all steps from base system setup are done > - # This constant and constant in rest-service -> basesystem.rb are not connected > - # and do not have to be the same value > FINAL_STEP = "FINISH" > # nextstep and backstep expect, that wizard session variables are set > def ensure_wizard > @@ -62,8 +60,9 @@ class ControlpanelController < ApplicationController > # basesystem.save will set always current to FINAL_STEP > basesystem.save > session[:wizard_current] = FINAL_STEP > + redirect_to "/controlpanel" > end > - redirect_to "/controlpanel" > + redirect_to get_redirect_hash(session[:wizard_current]) > end Hi, you cannot do what this change (I already fix it, so this is just note). You redirect_to /controlpanel if basesystem steps is done and then you again redirect to last wizard step, which broke system. You can look at my commit for correct behavior ( only one redirect per action!) -- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, webyast modules language and time -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

1 0

[yast-devel] automatic package submission should be working again
by Duncan Mac-Vicar P. 02 Sep '09

02 Sep '09

Lets see if YaST:Web builds by tomorrow ;-) Duncan -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

1 0

[yast-devel] yast-commit mails delayed by almost a week ?!
by Klaus Kaempf 02 Sep '09

02 Sep '09

Hi, I'm wondering if others also get yast-commit mails delayed by almost a week. E.g. I received this morning a commit from last week Thursday. I just opened bnc#536189 to track this down. Klaus --- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

2 3

[yast-devel] Re: [yast-commit] <web-client> master : status module: code cleanup. TODO: implement limit format with maximum and minimum
by josef reidinger 02 Sep '09

02 Sep '09

Bjoern Geuken napsal(a): > ref: refs/heads/master > commit 995bc764b3e72bdb385961a80e3ee861fd9edec4 > Author: Bjoern Geuken <bgeuken(a)suse.de> > Date: Wed Sep 2 10:45:36 2009 +0200 > > status module: code cleanup. TODO: implement limit format with maximum and minimum > --- > .../status/app/controllers/status_controller.rb | 100 +++++--------------- > 1 files changed, 24 insertions(+), 76 deletions(-) > > diff --git a/plugins/status/app/controllers/status_controller.rb b/plugins/status/app/controllers/status_controller.rb > index bd5d22c..d791323 100644 > --- a/plugins/status/app/controllers/status_controller.rb > +++ b/plugins/status/app/controllers/status_controller.rb > @@ -14,63 +14,35 @@ class StatusController < ApplicationController > @permissions = @client.permissions > end > > - def create_data_map( tree, label = "") > + def write_data_group(label, group, metric_name, limits=nil) > + data_list = Array.new > + divisor = (group == "memory")? 1024*1024 : 1 # take MByte for the value > + count = 0 > + data_list = label.map{ |v| count+=1; [count,v.to_f/divisor]} Hi, just few notes, here I don't understand what is label. You have default value for label empty string "", is it really intended? then this map take from String each character and convert it to float (so you get ascii value). From my point of view maybe label is array? > + @data_group[group].merge!({metric_name => data_list}) As I previously say, merge is if you want merge two hash, but in this case you want set one value in hash (it is more effective and less confusing) so you can use @data_group[group][metric_name] = data_list > + divisor = (group == "memory")? 1024*1024 : 1 # take MByte for the value you redefine divisor with same value, is it intended? (second line of this method) Moving shared function to separete method and use it to reduce code (DRY) is good job ;) Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

1 0

[yast-devel] Don't use sudo git...
by Björn Geuken 01 Sep '09

01 Sep '09

Hi, I accidently send a patch with Author: root <root(a)linux-qwou.(none)> by using sudo. I appologize for any confusion and would be happy if anyone could help fixing this. thanks Björn -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

2 1

[yast-devel] web client, errors and user interface
by Duncan Mac-Vicar Prett 01 Sep '09

01 Sep '09

I have commited Error handling improved for the web client See http://en.opensuse.org/YaST/Web/Development/Error_Handling for more information. New features: - unhandled ajax errors are displayed in the standard flash messages facility, using standard jquery.ui styles, and nothing is rendered - provide helper report_error to put an exception into the flash message area, mean to be used for handled errors. This allows an ajax request to report the error but still render something. - normal handled errors can still be reported using standard Rails flash[] facility. - popup to examine backtrace and copy to clipboard facility As we put the errors in the flash zone, this follows http://en.opensuse.org/YaST/Web/Docu/Style_Guide#Messages What needs still to be done: - We use the standard jquery.ui style, there are some glitches like the flash messages end to close to the status block. The details popup needs to be resizable and may be bigger. - transparent integration with known service errors. Right now we handle normal catched errors and uncatched ones. However for problems like a permission problem, there is no need for every action to worry about them, but it is a too common problem for the general exception trap. So either needs to be handled in a common place, using the information that the service will give to the client, or handled also by the common trap, but having more specific information to show a more specific template and possible solutions. This could be achieved for example by making ServiceResource to throw a more concrete error in this case, and then making the rescue action to handle that (it could also recognize the normal ActiveResource permission denied exception). -- Duncan Mac-Vicar P. - Engineering Manager, YaST SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg) -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

1 0

[yast-devel] concept of messages in webYaST
by Martin Schmidkunz 01 Sep '09

01 Sep '09

Hi, please find a first draft concerning the use of messages in webYaST at: http://en.opensuse.org/YaST/Web/Docu/Style_Guide#Messages The basic idea is to distinguish between questions and error messages: questions will be presented as pop ups and error messages within the dialog (between header and content) Furhtermore the idea is to simplify things for developers and users and to skip any destinction between error serverities: there will be just one way to display errors. This doesn`t make it necessary any more to gamble whether a possible error is a warning or a critial one. This is also a concept which was already introduced by Apple. Informative messages should be displayed in the place where the user has his focus (e.g. a loading icon in the row where the information is to be displayed and not on the top of the content because that would mean that the content is moved down everytime a simple loading process happens). Similar case is how to deal deal with questions: there a distinction between informative and warning type can be skippes as well because the user is interrupted in the workflow in any case and by reducing the amount of information in the question dialog, the user is supported to focus on the question text. What do you think about that concept? Cu, Martin --- Martin Schmidkunz User Experience Specialist mschmidkunz(a)novell.com +49 (0) 911 740 53-346 --- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg) --- Novell® Making IT Work As One™

1 0

[yast-devel] WebYaST: package versioning?
by Jiří Suchomel 01 Sep '09

01 Sep '09

Hi, I've just realized that most of traditional yast2 packages present in WebYaST repository have the versions of 2.18.*. But this is versioning based on trunk, should we have there the versions of patches SLE11 packages (2.17.<sles-11-released>++), instead of the trunk ones? That's not only about versioning, but also about the non-webyast related changes in these packages; I assume there should be no new stuff between SLE11 release and now, except the WebYaST related one... Jiri -- Jiri Suchomel SUSE LINUX, s.r.o. e-mail: jsuchome(a)suse.cz Lihovarská 1060/12 tel: +420 284 028 960 190 00 Praha 9, Czech Republic http://www.suse.cz -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

2 2

[yast-devel] Re: [yast-commit] <rest-service> network : fixed test
by Klaus Kaempf 01 Sep '09

01 Sep '09

* Michal Zugec <mzugec(a)suse.cz> [Aug 31. 2009 13:13]: > @@ -13,8 +13,10 @@ class RoutesControllerTest < Test::Unit::TestCase > # to extract routes data from all YaPI map > # > def test_show > - Routes.expects(:find).returns({:routes=>{:default=>'10.20.30.40'}}) > - Routes.find > + # if this code raises, test-unit 2.0.3 and mocha 0.9.5 fail > + # with PASSTHROUGH_EXCEPTIONS. version mismatch?! > + Route.expects(:find).returns({:routes=>{:default=>{'via'=>'10.20.30.40'}}}) > + Route.find Can you explain what you're actually trying to test here ? Klaus --- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

2 1

[yast-devel] Re: [yast-commit] <rest-service> master : status module: fixed evaluation of intervals for metrics
by Josef Reidinger 01 Sep '09

01 Sep '09

On 08/31/2009 12:49 PM, Bjoern Geuken wrote: > ref: refs/heads/master > commit 7fd7a8616e46b77b97d668b6782283864d32295b > Author: Bjoern Geuken <bgeuken(a)suse.de> > Date: Mon Aug 31 12:49:45 2009 +0200 > > status module: fixed evaluation of intervals for metrics > --- > plugins/status/lib/status.rb | 33 ++++++++++++------------------ > plugins/status/test/unit/status_test.rb | 3 +- > 2 files changed, 15 insertions(+), 21 deletions(-) > > diff --git a/plugins/status/lib/status.rb b/plugins/status/lib/status.rb > index 8628dde..58b7344 100644 > --- a/plugins/status/lib/status.rb > +++ b/plugins/status/lib/status.rb > @@ -9,9 +9,8 @@ class Status > :health_status, > :metrics, > :limits > -#=begin > + > def to_xml(options = {}) > -# puts @limits.inspect > xml = options[:builder] ||= Builder::XmlMarkup.new(options) > xml.instruct! unless options[:skip_instruct] > > @@ -20,7 +19,7 @@ class Status > data.each {|metric, data| > xml.metric(:name => metric, :metricgroup => metric_group) do > xml.starttime(@data[metric_group][metric]["starttime"]) > - xml.interval(5) > + xml.interval(@data[metric_group][metric]["interval"]) > data.each {|label,data| > unless label == "starttime" or label == "interval" > xml.label(:type => "hash", :name => label) do > @@ -201,43 +200,37 @@ class Status > end > unless labels.blank? > # set values for each label and time > + # evaluates interval and starttime once for each metric (not each label) > + nexttime = 9.9e+99 > + result["starttime"] = 9.9e+99 I suggest use some constant for this number because it is quite confusing (I think that it is some initial value, but maybe it is error indicator or some default. Constant give behavior to number and it is then easier to change it and test values on this value > lines.each do |l| # each time > unless l.blank? It is just my recomendation, but if you do big "if" statement in code and indent whole loop body, I think that if you just use next if l.blank? at begin of loop it increase readiness. (the best sulution is to write shorted functions or refactor it to separated parts - I recommend use reek or roodi to check your coding style) > if l =~ /\d*:\D*/ It looks like duplicite check, you check blank and then regex which is not blank, is really necessary to test blankness? and I miss at least some logging about bad input value. > pair = l.split ":" > values = pair[1].split " " > - #puts pair[1] > column = 0 > values.each do |v| # each label > - result["starttime"] ||= pair[0] > - result["starttime"] = pair[0] if pair[0].to_i < result["starttime"].to_i > - nexttime = > + if result["interval"].nil? > + if pair[0].to_i < result["starttime"].to_i > + result["starttime"] = pair[0].to_i > + elsif pair[0].to_i < nexttime and pair[0].to_i > result["starttime"].to_i > + nexttime = pair[0].to_i > + end > + end > > if v != "nan" #store valid values only > result["#{labels[column]}"] ||= Hash.new > -# result["#{labels[column]}"].merge!(v) > result["#{labels[column]}"].merge!({"#{pair[0].chomp(": ")}" => v}) I don't understand why you chomp there, as you have only digits in pair[0] ( regex ensure this ). Second really confusing code on this line is merge. It is really useless. if you use result["#{labels[column]}"][pair[0]] = v you get same result. Another think is that you reusing "#{labels[column]}". At first you can save it samewhere like label = "#{labels[column]}" At second I think that this string is really confusing, why you don't use just labels[column] ? and if it is not String, then add .to_s ? > - #result["#{labels[column]}"].merge!({"T_#{pair[0].chomp(": ")}" => v}) > column += 1 > else > result["#{labels[column]}"] ||= Hash.new > -# result["#{labels[column]}"].merge!("invalid") > result["#{labels[column]}"].merge!({"#{pair[0].chomp(": ")}" => "invalid"}) Same as above and you don't increase column, so if you get invalid value and then get valid value you with same pair[0] you overwrite invalid value. Is it intended to increase column only for valid data? It looks little confusing for me, as if status is invalid (cannot fetch data etcc) then I think it should show or hide this column, but next measure should be in next column not? > end > end > end > end > end > - #setting the limits > -# result.each do |key, value| > -# path = rrdfile[datapath.length+1..rrdfile.length-1].chomp('.rrd') > -# path +="/" + key if key!="value" #do not take care about the value flag > -# path = path.tr('-','_') > -# if @limits.has_key?(path) > -# result[key] ||= Hash.new > -# result[key].merge!({"limit" => @limits[path] }) > -# end > -# end > + result["interval"] = nexttime.to_i - result["starttime"].to_i if result["interval"].nil? testing for nil? is automatic so you can use result["interval"] = nexttime.to_i - result["starttime"].to_i unless result["interval"] If you have any question feel free to ask. -- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, webyast modules language and time -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

1 0