)]}'
{"lib/puppet/provider/keystone.rb":[{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"ab290cd470c07fb161b108c21bad15df67ebf7b6","unresolved":false,"context_lines":[{"line_number":37,"context_line":"  def self.default_domain_id"},{"line_number":38,"context_line":"    return @default_domain_id if @default_domain_id"},{"line_number":39,"context_line":"    if keystone_file and keystone_file[\u0027identity\u0027] and keystone_file[\u0027identity\u0027][\u0027default_domain_id\u0027]"},{"line_number":40,"context_line":"      @default_domain_id \u003d \"#{keystone_file[\u0027identity\u0027][\u0027default_domain_id\u0027].strip}\""},{"line_number":41,"context_line":"    else"},{"line_number":42,"context_line":"      @default_domain_id \u003d \u0027default\u0027"},{"line_number":43,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_36a056a5","line":40,"updated":"2015-09-09 16:39:11.000000000","message":"This could also be rewritten to use get_section.","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"ebdda1ed1304efad204780ec436464887f8d6f86","unresolved":false,"context_lines":[{"line_number":37,"context_line":"  def self.default_domain_id"},{"line_number":38,"context_line":"    return @default_domain_id if @default_domain_id"},{"line_number":39,"context_line":"    if keystone_file and keystone_file[\u0027identity\u0027] and keystone_file[\u0027identity\u0027][\u0027default_domain_id\u0027]"},{"line_number":40,"context_line":"      @default_domain_id \u003d \"#{keystone_file[\u0027identity\u0027][\u0027default_domain_id\u0027].strip}\""},{"line_number":41,"context_line":"    else"},{"line_number":42,"context_line":"      @default_domain_id \u003d \u0027default\u0027"},{"line_number":43,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_fba6340e","line":40,"in_reply_to":"da20952f_36a056a5","updated":"2015-09-10 01:50:21.000000000","message":"Can be done in master in a new patch, no blocker","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"c0097f97d7e32749e185ebd28c59529262b2ab6a","unresolved":false,"context_lines":[{"line_number":86,"context_line":"  end"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"  def self.get_section(group, name)"},{"line_number":89,"context_line":"    if keystone_file \u0026\u0026 keystone_file[group] \u0026\u0026 keystone_file[\u0027DEFAULT\u0027][name]"},{"line_number":90,"context_line":"      return keystone_file[group][name].strip"},{"line_number":91,"context_line":"    end"},{"line_number":92,"context_line":"    return nil"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_f6565ecd","line":89,"updated":"2015-09-09 16:31:36.000000000","message":"It should be keystone_file[group][name], no ?","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d4fc42d47a6a4a235673e477b559085b20451533","unresolved":false,"context_lines":[{"line_number":86,"context_line":"  end"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"  def self.get_section(group, name)"},{"line_number":89,"context_line":"    if keystone_file \u0026\u0026 keystone_file[group] \u0026\u0026 keystone_file[\u0027DEFAULT\u0027][name]"},{"line_number":90,"context_line":"      return keystone_file[group][name].strip"},{"line_number":91,"context_line":"    end"},{"line_number":92,"context_line":"    return nil"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_9b1ff808","line":89,"in_reply_to":"da20952f_56bceada","updated":"2015-09-10 01:56:30.000000000","message":"This is ok \n1) This is a backport - cannot change a backport unless git conflict\n2) The only places where get_section is used, \u0027DEFAULT\u0027 is the group value.","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"45d8d2f4405d0ad402c07cfd10cf6a6e8548f946","unresolved":false,"context_lines":[{"line_number":86,"context_line":"  end"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"  def self.get_section(group, name)"},{"line_number":89,"context_line":"    if keystone_file \u0026\u0026 keystone_file[group] \u0026\u0026 keystone_file[\u0027DEFAULT\u0027][name]"},{"line_number":90,"context_line":"      return keystone_file[group][name].strip"},{"line_number":91,"context_line":"    end"},{"line_number":92,"context_line":"    return nil"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_5bf080c9","line":89,"in_reply_to":"da20952f_56bceada","updated":"2015-09-10 01:58:14.000000000","message":"yes that\u0027s a bug, thanks @sofer for spotting it.\n\nHopefully  we\u0027re not hitting it because all the calls to get_section are made using \u0027DEFAULT\u0027 for group.\n\nI\u0027ll make a fix for master.","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"ab290cd470c07fb161b108c21bad15df67ebf7b6","unresolved":false,"context_lines":[{"line_number":86,"context_line":"  end"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"  def self.get_section(group, name)"},{"line_number":89,"context_line":"    if keystone_file \u0026\u0026 keystone_file[group] \u0026\u0026 keystone_file[\u0027DEFAULT\u0027][name]"},{"line_number":90,"context_line":"      return keystone_file[group][name].strip"},{"line_number":91,"context_line":"    end"},{"line_number":92,"context_line":"    return nil"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_56bceada","line":89,"in_reply_to":"da20952f_f6565ecd","updated":"2015-09-09 16:39:11.000000000","message":"ack","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"7187290cc1269f473576db592a90310346752e37","unresolved":false,"context_lines":[{"line_number":160,"context_line":"  end"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"  def self.ssl?"},{"line_number":163,"context_line":"    if keystone_file \u0026\u0026 keystone_file[\u0027ssl\u0027] \u0026\u0026 keystone_file[\u0027ssl\u0027][\u0027enable\u0027] \u0026\u0026 keystone_file[\u0027ssl\u0027][\u0027enable\u0027].strip.downcase \u003d\u003d \u0027true\u0027"},{"line_number":164,"context_line":"      return true"},{"line_number":165,"context_line":"    end"},{"line_number":166,"context_line":"    return false"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_2a59c635","line":163,"updated":"2015-09-09 16:48:06.000000000","message":"Yes that\u0027s what I though (ie, a theorical problem mainly), but I figured I raised the point anyway.","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"c0097f97d7e32749e185ebd28c59529262b2ab6a","unresolved":false,"context_lines":[{"line_number":160,"context_line":"  end"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"  def self.ssl?"},{"line_number":163,"context_line":"    if keystone_file \u0026\u0026 keystone_file[\u0027ssl\u0027] \u0026\u0026 keystone_file[\u0027ssl\u0027][\u0027enable\u0027] \u0026\u0026 keystone_file[\u0027ssl\u0027][\u0027enable\u0027].strip.downcase \u003d\u003d \u0027true\u0027"},{"line_number":164,"context_line":"      return true"},{"line_number":165,"context_line":"    end"},{"line_number":166,"context_line":"    return false"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_165af2f2","line":163,"updated":"2015-09-09 16:31:36.000000000","message":"You could do \n  \n  if is_ssl \u003d get_section(\u0027ssl\u0027, \u0027enable\u0027)\n    return true if is_ssl.downcase \u003d\u003d \u0027true\u0027\n\nIt should be noted here that the Boolean type in openstack config accept on/off, 1/0, yes/no, not just true/false (case insensitive for all): https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L108-L116\nbut maybe it should be part of another review, or I missed something.","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"ab290cd470c07fb161b108c21bad15df67ebf7b6","unresolved":false,"context_lines":[{"line_number":160,"context_line":"  end"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"  def self.ssl?"},{"line_number":163,"context_line":"    if keystone_file \u0026\u0026 keystone_file[\u0027ssl\u0027] \u0026\u0026 keystone_file[\u0027ssl\u0027][\u0027enable\u0027] \u0026\u0026 keystone_file[\u0027ssl\u0027][\u0027enable\u0027].strip.downcase \u003d\u003d \u0027true\u0027"},{"line_number":164,"context_line":"      return true"},{"line_number":165,"context_line":"    end"},{"line_number":166,"context_line":"    return false"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_36b5b6cf","line":163,"in_reply_to":"da20952f_165af2f2","updated":"2015-09-09 16:39:11.000000000","message":"This is problematic in general, in all puppet module code.  What we really need is a port of oslo.config to ruby, then create a puppet provider for that.\n\nBut in practice, it is usually not a problem.  Since puppet is used to write the config files, it usually uses [Tt]rue [Ff]alse strings for boolean values, which are then read correctly as booleans by other puppet code.","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"}],"spec/unit/provider/keystone_spec.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"c0097f97d7e32749e185ebd28c59529262b2ab6a","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    end"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    it \u0027should be false if ssl is configured and disable in keystone file\u0027 do"},{"line_number":43,"context_line":"      mock \u003d {\u0027ssl\u0027 \u003d\u003e {\u0027enable\u0027 \u003d\u003e \u0027False\u0027}}"},{"line_number":44,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(true)"},{"line_number":45,"context_line":"      Puppet::Util::IniConfig::File.expects(:new).returns(mock)"},{"line_number":46,"context_line":"      mock.expects(:read).with(\u0027/etc/keystone/keystone.conf\u0027)"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_b671c679","line":43,"updated":"2015-09-09 16:31:36.000000000","message":"Here for instance we can have 0, or off, no.","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"ebdda1ed1304efad204780ec436464887f8d6f86","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    end"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    it \u0027should be false if ssl is configured and disable in keystone file\u0027 do"},{"line_number":43,"context_line":"      mock \u003d {\u0027ssl\u0027 \u003d\u003e {\u0027enable\u0027 \u003d\u003e \u0027False\u0027}}"},{"line_number":44,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(true)"},{"line_number":45,"context_line":"      Puppet::Util::IniConfig::File.expects(:new).returns(mock)"},{"line_number":46,"context_line":"      mock.expects(:read).with(\u0027/etc/keystone/keystone.conf\u0027)"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_9bae18e6","line":43,"in_reply_to":"da20952f_361136b0","updated":"2015-09-10 01:50:21.000000000","message":"Can be done in master in a new patch, no blocker","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"ab290cd470c07fb161b108c21bad15df67ebf7b6","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    end"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    it \u0027should be false if ssl is configured and disable in keystone file\u0027 do"},{"line_number":43,"context_line":"      mock \u003d {\u0027ssl\u0027 \u003d\u003e {\u0027enable\u0027 \u003d\u003e \u0027False\u0027}}"},{"line_number":44,"context_line":"      File.expects(:exists?).with(\"/etc/keystone/keystone.conf\").returns(true)"},{"line_number":45,"context_line":"      Puppet::Util::IniConfig::File.expects(:new).returns(mock)"},{"line_number":46,"context_line":"      mock.expects(:read).with(\u0027/etc/keystone/keystone.conf\u0027)"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"da20952f_361136b0","line":43,"in_reply_to":"da20952f_b671c679","updated":"2015-09-09 16:39:11.000000000","message":"see my other note about this issue","commit_id":"8442bfc635575aa67be5b867ad783da5b67609cc"}]}
