thanks marios. I made the adjustment per your suggestions and committed. --SM
On Mon, Jul 11, 2011 at 5:34 AM, [email protected] <[email protected]>wrote: > ACK - I like this approach; an earlier version was setting ID like: > > type=group&owner=123456789012&**name=new_firewall|type=** > address&family=ipv4&address=**192.168.1.1&prefix=24" > > i.e. each value was prefixed with its field name but this made the ID > string too long. I think your approach of building the ID by explicitly > setting positions is safer. > > Can you please look at a couple formatting nits before you commit (you have > commit right now I believe?): > > * trailing whitespaces lines 78, 90 92 of the patch (see David > Lutterkort's email "Whitespace and you" on 03rd June 2011 > http://mail-archives.apache.**org/mod_mbox/incubator-** > deltacloud-dev/201106.mbox/%**3C1307092263.2654.19.camel@** > melon.watzmann.net%3E<http://mail-archives.apache.org/mod_mbox/incubator-deltacloud-dev/201106.mbox/%[email protected]%3E>for > editor settings etc - e.g. I couldn't apply your patch due to my git > pre-commit hooks about whitespace (I edited the patch and then applied)) > > * can you use spaces instead of tabs lines 79, 80, 82 of the patch (i know > this sounds pedantic but different editors handle tabs in different ways so > formatting gets messed around - the dev team uses 2 spaces/tab) > > * why do you prefer "sources_string.slice(0,**sources_string.length-1)" > over "sources_string.chomp!(",")" > > marios > > > On 10/07/11 20:28, [email protected] wrote: > >> From: Sang-Min Park<[email protected]> >> >> --- >> server/lib/deltacloud/drivers/**ec2/ec2_driver.rb | 12 ++++++------ >> 1 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/server/lib/deltacloud/**drivers/ec2/ec2_driver.rb >> b/server/lib/deltacloud/**drivers/ec2/ec2_driver.rb >> index 53f95b6..a21ba34 100644 >> --- a/server/lib/deltacloud/**drivers/ec2/ec2_driver.rb >> +++ b/server/lib/deltacloud/**drivers/ec2/ec2_driver.rb >> @@ -809,15 +809,15 @@ module Deltacloud >> #generate uid from firewall rule parameters (amazon doesn't do >> this for us >> def firewall_rule_id(user_id, protocol, from_port, to_port, >> sources) >> sources_string = "" >> - sources.each do |source| >> - sources_string<<"@" >> - source.each_pair do |key,value| >> - sources_string<< "#{value}," >> + sources.each do |source| >> + if source[:type].to_s == "group" >> + sources_string<< "@#{source[:type]},#{source[:** >> owner]},#{source[:name]}," >> + else >> + sources_string<< "@#{source[:type]},#{source[:** >> family]},#{source[:address]},#**{source[:prefix]}," >> end >> - sources_string.chomp!(",") >> end >> #sources_string is >> @group,297467797945,test@**address,ipv4,10.1.1.1,24 >> etc >> - id_string = "#{user_id}~#{protocol}~#{** >> from_port}~#{to_port}~#{**sources_string}" >> + id_string = "#{user_id}~#{protocol}~#{** >> from_port}~#{to_port}~#{**sources_string.slice(0,** >> sources_string.length-1)}" >> end >> >> #extract params from uid >> > > -- ---------------------------------------------------- Sang-Min Park Engineer Eucalyptus Systems
