)]}'
{"lib/puppet/provider/keystone.rb":[{"author":{"_account_id":1607,"name":"Yu xingchao","email":"yuxcer@gmail.com","username":"yuxcer"},"change_message_id":"046ea7cbdb568353d639f8919396d504851ad0a6","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    return auth_url"},{"line_number":90,"context_line":"  end"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"  def self.get_config(group, name)"},{"line_number":93,"context_line":"    if keystone_file \u0026\u0026 keystone_file[group] \u0026\u0026 keystone_file[\u0027DEFAULT\u0027][name]"},{"line_number":94,"context_line":"      return keystone_file[group][name].strip"},{"line_number":95,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":7,"id":"fa1b9901_cdd48621","line":92,"updated":"2015-08-20 08:22:31.000000000","message":"Just a suggestion about option naming: use section to take place of group will make this option more clear.","commit_id":"cbe6d62da690fef55648882bc42da483829974cb"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"6c0b2629ac1f794bf9139f8557e15387ad2659cb","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    return auth_url"},{"line_number":90,"context_line":"  end"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"  def self.get_config(group, name)"},{"line_number":93,"context_line":"    if keystone_file \u0026\u0026 keystone_file[group] \u0026\u0026 keystone_file[\u0027DEFAULT\u0027][name]"},{"line_number":94,"context_line":"      return keystone_file[group][name].strip"},{"line_number":95,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":7,"id":"fa1b9901_4dccf669","line":92,"in_reply_to":"fa1b9901_cdd48621","updated":"2015-08-20 11:20:32.000000000","message":"That would be a better name, will do, thanks","commit_id":"cbe6d62da690fef55648882bc42da483829974cb"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"4e7ce8bf78b101ac506b14da58f0c0413e4d1f6b","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"  def self.get_admin_endpoint"},{"line_number":58,"context_line":"    endpoint \u003d nil"},{"line_number":59,"context_line":"    if keystone_file"},{"line_number":60,"context_line":"      if url \u003d get_section(\u0027DEFAULT\u0027, \u0027admin_endpoint\u0027)"},{"line_number":61,"context_line":"        endpoint \u003d url.chomp(\u0027/\u0027)"},{"line_number":62,"context_line":"      else"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_e3a7f5ba","line":59,"updated":"2015-08-20 20:18:54.000000000","message":"What does keystone_file do?  You use it in the if clause, then you never use it again?  Why does there have to be a get_section method?  What\u0027s wrong with just using keystone_file?  I don\u0027t understand the logical necessity of this change.\n\nWhy can\u0027t we just fix the url version issue without all of this other refactor/moving code around?","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c0d164eb5dc961ceb7f7f0c2530500cde3710d31","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"  def self.get_admin_endpoint"},{"line_number":58,"context_line":"    endpoint \u003d nil"},{"line_number":59,"context_line":"    if keystone_file"},{"line_number":60,"context_line":"      if url \u003d get_section(\u0027DEFAULT\u0027, \u0027admin_endpoint\u0027)"},{"line_number":61,"context_line":"        endpoint \u003d url.chomp(\u0027/\u0027)"},{"line_number":62,"context_line":"      else"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_559e90b9","line":59,"in_reply_to":"fa1b9901_e3a7f5ba","updated":"2015-08-21 02:21:43.000000000","message":"\u003e What does kesytone_file do? \n\nkeystone_file makes sure the file exist, the previous version was flawed:\n\nPlease tell me what\u0027s happening to the line  \n\"#{protocol}://#{host}:#{admin_port}/v#{@credentials.version}/\"\nafter the if keystone_file statement in the original code?\n\n\u003e Why does there have to be a get_section method? \n\nThe get_section method takes care of returning the value from the variable in related section. This is a way to make the code easier to read, as part of the ruby philosphy (and I hope not only!) make small methods. I believe the whole point is maintainability.\n\n\u003e Why can\u0027t we just fix the url version issue without all of this other refactor/moving code around?\n\nBecause the legacy code needs it.\nI initially had a big patch but that\u0027s not a good way of pushing reviews, too big, too complicated. Small steps, here we come.\n\nNow, this patch doesn\u0027t pretend to fix url version issues but it should help.\nThe url version issues are mostly fixed in Liberty because we can go version-less, at least for the Endpoints\u0027 URL.\nThat\u0027s the whole point of this patch, makes things easier.\nNow for Kilo, it\u0027s not fixing anything but it\u0027s hurting either.","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"a4c3d4d09d6a67c71810397c94874229e61f93f4","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        cfg_host \u003d get_section(\u0027DEFAULT\u0027, \u0027admin_bind_host\u0027)"},{"line_number":67,"context_line":"        cfg_host \u003d clean_host(cfg_host)"},{"line_number":68,"context_line":"        host \u003d cfg_host.nil? ? \u0027127.0.0.1\u0027 : cfg_host"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        protocol \u003d ssl? ? \u0027https\u0027 : \u0027http\u0027"},{"line_number":71,"context_line":"        endpoint \u003d \"#{protocol}://#{host}:#{admin_port}\""}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_089328fb","line":68,"updated":"2015-08-20 20:47:00.000000000","message":"would be nice to handle this in the clean_host method instead, though it cannot return nil, IIUC. And cover this case in the unit tests as well.","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c0d164eb5dc961ceb7f7f0c2530500cde3710d31","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        cfg_host \u003d get_section(\u0027DEFAULT\u0027, \u0027admin_bind_host\u0027)"},{"line_number":67,"context_line":"        cfg_host \u003d clean_host(cfg_host)"},{"line_number":68,"context_line":"        host \u003d cfg_host.nil? ? \u0027127.0.0.1\u0027 : cfg_host"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        protocol \u003d ssl? ? \u0027https\u0027 : \u0027http\u0027"},{"line_number":71,"context_line":"        endpoint \u003d \"#{protocol}://#{host}:#{admin_port}\""}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_753dd48d","line":68,"in_reply_to":"fa1b9901_089328fb","updated":"2015-08-21 02:21:43.000000000","message":"I thought about that too but I wanted the default values to be clearly visible.\nI guess it should be the default anyway, so I\u0027ll do it. Thanks","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"}],"spec/classes/keystone_spec.rb":[{"author":{"_account_id":3153,"name":"Emilien Macchi","email":"emilien@redhat.com","username":"emilienm"},"change_message_id":"a85fc00bbabe5cbdef5612c75b653939b83be7c2","unresolved":false,"context_lines":[{"line_number":27,"context_line":"      \u0027admin_bind_host\u0027                     \u003d\u003e \u00270.0.0.0\u0027,"},{"line_number":28,"context_line":"      \u0027public_port\u0027                         \u003d\u003e \u00275000\u0027,"},{"line_number":29,"context_line":"      \u0027admin_port\u0027                          \u003d\u003e \u002735357\u0027,"},{"line_number":30,"context_line":"      \u0027admin_token\u0027                         \u003d\u003e \u0027service_token\u0027,"},{"line_number":31,"context_line":"      \u0027verbose\u0027                             \u003d\u003e false,"},{"line_number":32,"context_line":"      \u0027debug\u0027                               \u003d\u003e false,"},{"line_number":33,"context_line":"      \u0027use_stderr\u0027                          \u003d\u003e true,"}],"source_content_type":"text/x-ruby","patch_set":5,"id":"fa1b9901_5a12d863","side":"PARENT","line":30,"updated":"2015-08-18 12:34:57.000000000","message":"why?","commit_id":"d8cb5e2a2c7bebe7c39fe2b55d4a1d4bf6128ae7"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"82603dacff915ca60f282278cf82846741a3f7df","unresolved":false,"context_lines":[{"line_number":27,"context_line":"      \u0027admin_bind_host\u0027                     \u003d\u003e \u00270.0.0.0\u0027,"},{"line_number":28,"context_line":"      \u0027public_port\u0027                         \u003d\u003e \u00275000\u0027,"},{"line_number":29,"context_line":"      \u0027admin_port\u0027                          \u003d\u003e \u002735357\u0027,"},{"line_number":30,"context_line":"      \u0027admin_token\u0027                         \u003d\u003e \u0027service_token\u0027,"},{"line_number":31,"context_line":"      \u0027verbose\u0027                             \u003d\u003e false,"},{"line_number":32,"context_line":"      \u0027debug\u0027                               \u003d\u003e false,"},{"line_number":33,"context_line":"      \u0027use_stderr\u0027                          \u003d\u003e true,"}],"source_content_type":"text/x-ruby","patch_set":5,"id":"fa1b9901_59d731ed","side":"PARENT","line":30,"in_reply_to":"fa1b9901_5a12d863","updated":"2015-08-18 19:52:15.000000000","message":"Because it\u0027s already defined in line #23 and it triggers a warning.","commit_id":"d8cb5e2a2c7bebe7c39fe2b55d4a1d4bf6128ae7"}],"spec/unit/provider/keystone_spec.rb":[{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"a4c3d4d09d6a67c71810397c94874229e61f93f4","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    it \u0027should be false if ssl is there is no keystone file\u0027 do"},{"line_number":30,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(false)"},{"line_number":31,"context_line":"      expect(klass.ssl?).to be_falsey"},{"line_number":32,"context_line":"    end"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    it \u0027should be false if ssl is not configured in keystone file\u0027 do"},{"line_number":35,"context_line":"      mock \u003d {}"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_833ab9e5","line":32,"updated":"2015-08-20 20:47:00.000000000","message":"... if there is no ...","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c0d164eb5dc961ceb7f7f0c2530500cde3710d31","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    it \u0027should be false if ssl is there is no keystone file\u0027 do"},{"line_number":30,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(false)"},{"line_number":31,"context_line":"      expect(klass.ssl?).to be_falsey"},{"line_number":32,"context_line":"    end"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    it \u0027should be false if ssl is not configured in keystone file\u0027 do"},{"line_number":35,"context_line":"      mock \u003d {}"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_b54fec32","line":32,"in_reply_to":"fa1b9901_833ab9e5","updated":"2015-08-21 02:21:43.000000000","message":"Done","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"a4c3d4d09d6a67c71810397c94874229e61f93f4","unresolved":false,"context_lines":[{"line_number":118,"context_line":"    end"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    it \u0027should handle an admin_endpoint with a trailing slash\u0027 do"},{"line_number":121,"context_line":"      mock \u003d {\u0027DEFAULT\u0027 \u003d\u003e {\u0027admin_endpoint\u0027 \u003d\u003e \u0027https://keystone.example.com/v3\u0027 }, \u0027ssl\u0027 \u003d\u003e {\u0027enable\u0027 \u003d\u003e \u0027False\u0027}}"},{"line_number":122,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(true)"},{"line_number":123,"context_line":"      Puppet::Util::IniConfig::File.expects(:new).returns(mock)"},{"line_number":124,"context_line":"      mock.expects(:read).with(\u0027/etc/keystone/keystone.conf\u0027)"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_7e57fec9","line":121,"updated":"2015-08-20 20:47:00.000000000","message":"what if .../v3/ ?","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c0d164eb5dc961ceb7f7f0c2530500cde3710d31","unresolved":false,"context_lines":[{"line_number":118,"context_line":"    end"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    it \u0027should handle an admin_endpoint with a trailing slash\u0027 do"},{"line_number":121,"context_line":"      mock \u003d {\u0027DEFAULT\u0027 \u003d\u003e {\u0027admin_endpoint\u0027 \u003d\u003e \u0027https://keystone.example.com/v3\u0027 }, \u0027ssl\u0027 \u003d\u003e {\u0027enable\u0027 \u003d\u003e \u0027False\u0027}}"},{"line_number":122,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(true)"},{"line_number":123,"context_line":"      Puppet::Util::IniConfig::File.expects(:new).returns(mock)"},{"line_number":124,"context_line":"      mock.expects(:read).with(\u0027/etc/keystone/keystone.conf\u0027)"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_9502e8b2","line":121,"in_reply_to":"fa1b9901_7e57fec9","updated":"2015-08-21 02:21:43.000000000","message":"Actually this test is redundant with \u0027should use the admin_endpoint from keystone config file\u0027. But it does need to test the trailing slash too.","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"a4c3d4d09d6a67c71810397c94874229e61f93f4","unresolved":false,"context_lines":[{"line_number":122,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(true)"},{"line_number":123,"context_line":"      Puppet::Util::IniConfig::File.expects(:new).returns(mock)"},{"line_number":124,"context_line":"      mock.expects(:read).with(\u0027/etc/keystone/keystone.conf\u0027)"},{"line_number":125,"context_line":"      expect(klass.get_admin_endpoint).to eq(\u0027https://keystone.example.com/v3\u0027)"},{"line_number":126,"context_line":"    end"},{"line_number":127,"context_line":"  end"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_de0baad4","line":125,"updated":"2015-08-20 20:47:00.000000000","message":"should be http as you put ssl enable\u003d\u003eFalse","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c0d164eb5dc961ceb7f7f0c2530500cde3710d31","unresolved":false,"context_lines":[{"line_number":122,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(true)"},{"line_number":123,"context_line":"      Puppet::Util::IniConfig::File.expects(:new).returns(mock)"},{"line_number":124,"context_line":"      mock.expects(:read).with(\u0027/etc/keystone/keystone.conf\u0027)"},{"line_number":125,"context_line":"      expect(klass.get_admin_endpoint).to eq(\u0027https://keystone.example.com/v3\u0027)"},{"line_number":126,"context_line":"    end"},{"line_number":127,"context_line":"  end"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_552f703e","line":125,"in_reply_to":"fa1b9901_de0baad4","updated":"2015-08-21 02:21:43.000000000","message":"You\u0027re right but at same time ssl is not tested when using the section admin_endpoint so that should go away.","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"a4c3d4d09d6a67c71810397c94874229e61f93f4","unresolved":false,"context_lines":[{"line_number":164,"context_line":"    end"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    it \u0027should return the OS_URL from the environment\u0027 do"},{"line_number":167,"context_line":"      ENV[\u0027OS_URL\u0027] \u003d \u0027http://127.0.0.1:5001/v3\u0027"},{"line_number":168,"context_line":"      expect(klass.get_service_url).to eq(\u0027http://127.0.0.1:5001/v3\u0027)"},{"line_number":169,"context_line":"    end"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_5e947a41","line":167,"updated":"2015-08-20 20:47:00.000000000","message":"what if /v3/ ?","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c0d164eb5dc961ceb7f7f0c2530500cde3710d31","unresolved":false,"context_lines":[{"line_number":164,"context_line":"    end"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    it \u0027should return the OS_URL from the environment\u0027 do"},{"line_number":167,"context_line":"      ENV[\u0027OS_URL\u0027] \u003d \u0027http://127.0.0.1:5001/v3\u0027"},{"line_number":168,"context_line":"      expect(klass.get_service_url).to eq(\u0027http://127.0.0.1:5001/v3\u0027)"},{"line_number":169,"context_line":"    end"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_27888fc4","line":167,"in_reply_to":"fa1b9901_5e947a41","updated":"2015-08-21 02:21:43.000000000","message":"We must pass whatever is in OS_URL","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"a4c3d4d09d6a67c71810397c94874229e61f93f4","unresolved":false,"context_lines":[{"line_number":168,"context_line":"      expect(klass.get_service_url).to eq(\u0027http://127.0.0.1:5001/v3\u0027)"},{"line_number":169,"context_line":"    end"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    it \u0027should use admin_endpoint when nothing else is available\u0027 do"},{"line_number":172,"context_line":"      ENV.clear"},{"line_number":173,"context_line":"      mock \u003d \u0027http://127.0.0.1:5001\u0027"},{"line_number":174,"context_line":"      klass.expects(:admin_endpoint).twice.returns(mock)"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_debc0abd","line":171,"updated":"2015-08-20 20:47:00.000000000","message":"should use admin_endpoint and guess v3 when ...","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c0d164eb5dc961ceb7f7f0c2530500cde3710d31","unresolved":false,"context_lines":[{"line_number":168,"context_line":"      expect(klass.get_service_url).to eq(\u0027http://127.0.0.1:5001/v3\u0027)"},{"line_number":169,"context_line":"    end"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    it \u0027should use admin_endpoint when nothing else is available\u0027 do"},{"line_number":172,"context_line":"      ENV.clear"},{"line_number":173,"context_line":"      mock \u003d \u0027http://127.0.0.1:5001\u0027"},{"line_number":174,"context_line":"      klass.expects(:admin_endpoint).twice.returns(mock)"}],"source_content_type":"text/x-ruby","patch_set":9,"id":"fa1b9901_07df8bd9","line":171,"in_reply_to":"fa1b9901_debc0abd","updated":"2015-08-21 02:21:43.000000000","message":"Done. Thanks","commit_id":"3a3daabf11b2c36bf21c6e4ecd5341b4657b74b2"}]}