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/%[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
