)]}'
{"lib/puppet/provider/keystone.rb":[{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"dcc1efb3c32ee1aedfa13ef6919f0deda867f553","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    end"},{"line_number":196,"context_line":"    domain \u003d fetch_domain(domain_name)"},{"line_number":197,"context_line":"    domain_id \u003d domain ? domain[:id] : nil"},{"line_number":198,"context_line":"    if domain_id \u003d\u003d default_domain_id"},{"line_number":199,"context_line":"      name"},{"line_number":200,"context_line":"    else"},{"line_number":201,"context_line":"      \"#{name}::#{domain_name}\""}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_696ec802","line":198,"updated":"2015-09-28 15:31:10.000000000","message":"If domain_name is not found, then domain will be nil.\nIf domain is nil, then domain_id is nil.\nIf default_domain_id is not set, it will be nil.\n\nTherefore \"domain_does_not_exist\" \u003d\u003d \"Default\"\n\nI don\u0027t understand when fetch_domain will ever return nil.  This means you are referring to a domain before the keystone_domain resource has been evaluated?  Under what conditions can that happen?","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d640904da60aa407d5db8bfbcd6661117544cf2f","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    end"},{"line_number":196,"context_line":"    domain \u003d fetch_domain(domain_name)"},{"line_number":197,"context_line":"    domain_id \u003d domain ? domain[:id] : nil"},{"line_number":198,"context_line":"    if domain_id \u003d\u003d default_domain_id"},{"line_number":199,"context_line":"      name"},{"line_number":200,"context_line":"    else"},{"line_number":201,"context_line":"      \"#{name}::#{domain_name}\""}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_7c80adc0","line":198,"in_reply_to":"9a1a9d01_01afcee2","updated":"2015-09-29 01:40:23.000000000","message":"ok, right - default_domain_id should return \u0027default\u0027 if not set - it should never return nil.  So that\u0027s ok.\n\nOk.  So fetch_domain can return nil if the author of the manifest tries to use a domain that is not declared.  In this case, why not throw an exception?  Isn\u0027t that what puppet usually does when you try to use a resource that hasn\u0027t been declared?","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"b36e8ddc4d99fcf4a0ee07a0ae1f00c9d2b13904","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    end"},{"line_number":196,"context_line":"    domain \u003d fetch_domain(domain_name)"},{"line_number":197,"context_line":"    domain_id \u003d domain ? domain[:id] : nil"},{"line_number":198,"context_line":"    if domain_id \u003d\u003d default_domain_id"},{"line_number":199,"context_line":"      name"},{"line_number":200,"context_line":"    else"},{"line_number":201,"context_line":"      \"#{name}::#{domain_name}\""}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_01afcee2","line":198,"in_reply_to":"9a1a9d01_696ec802","updated":"2015-09-29 01:03:45.000000000","message":"\u003e If domain_name is not found, then domain will be nil. If domain is nil, then domain_id is nil. \n\nYes the caller could send a nil domain\nNow, I can the case where the domain is nil, only name should be returned that\u0027s a case missing. I\u0027ll add it.\n\n\u003eIf default_domain_id is not set, it will be nil.\n\nThat is not supposed to be possible, it\u0027s either \u0027default\u0027 value or set to something else.\n\n\u003e I don\u0027t understand when fetch_domain will ever return nil. This means you are referring to a domain before the keystone_domain resource has been evaluated? Under what conditions can that happen?\n\nIf a non existing domain is looked-up for then nil might be returned, the #fetch_domain to be able to handle it. Now if that never happens the better!","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"23109eeb19331350a9f3e9737e5e7add50b3ce75","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    end"},{"line_number":196,"context_line":"    domain \u003d fetch_domain(domain_name)"},{"line_number":197,"context_line":"    domain_id \u003d domain ? domain[:id] : nil"},{"line_number":198,"context_line":"    if domain_id \u003d\u003d default_domain_id"},{"line_number":199,"context_line":"      name"},{"line_number":200,"context_line":"    else"},{"line_number":201,"context_line":"      \"#{name}::#{domain_name}\""}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_6e5d4578","line":198,"in_reply_to":"9a1a9d01_7c80adc0","updated":"2015-09-29 12:10:13.000000000","message":"Yes, for raising an exception if a fetched domain returns nil sounds.\nWill do.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":7745,"name":"Denis Egorenko","email":"degorenko@mirantis.com","username":"degorenko"},"change_message_id":"8ddbc0edd632712ed8662abeb6ce45a7d45108a0","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    @domain_hash[id]"},{"line_number":66,"context_line":"  end"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"  def self.fetch_domain(domain)"},{"line_number":69,"context_line":"    request(\u0027domain\u0027, \u0027show\u0027, domain)"},{"line_number":70,"context_line":"  rescue Puppet::ExecutionFailure \u003d\u003e e"},{"line_number":71,"context_line":"    raise e unless e.message \u003d~ /No domain with a name or ID/"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_75c0fbe0","line":68,"updated":"2015-10-14 16:39:50.000000000","message":"May be we can reduce lines of code by using one common function e.q.  fetch_resource and by passing name of resource and args* as array? So, we can fill array in place where we run this function, like:\n\n  opts \u003d [some_option]\n  opts \u003c\u003c some_key_if_we_need \u003c\u003c value_for_key\n\nand so on\n\n  fetch_resource(\u0027name_of_resource\u0027, opts)","commit_id":"d2c44f731caa1ac3e57c407fc63c0c790e99e43f"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"a71ff2ceffe6101a4a205d381c7350398cb8a968","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    @domain_hash[id]"},{"line_number":66,"context_line":"  end"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"  def self.fetch_domain(domain)"},{"line_number":69,"context_line":"    request(\u0027domain\u0027, \u0027show\u0027, domain)"},{"line_number":70,"context_line":"  rescue Puppet::ExecutionFailure \u003d\u003e e"},{"line_number":71,"context_line":"    raise e unless e.message \u003d~ /No domain with a name or ID/"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"3a29b11f_80e4ac3b","line":68,"in_reply_to":"5a2ca52d_75c0fbe0","updated":"2015-10-19 04:16:17.000000000","message":"It\u0027s a good idea but I would rather have that taken care by an independent patch.","commit_id":"d2c44f731caa1ac3e57c407fc63c0c790e99e43f"}],"lib/puppet/provider/keystone_user/openstack.rb":[{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"dcc1efb3c32ee1aedfa13ef6919f0deda867f553","unresolved":false,"context_lines":[{"line_number":44,"context_line":"      # See if resource[:tenant_domain] or user specified as tenant::domain"},{"line_number":45,"context_line":"      tenant_name, tenant_domain \u003d self.class.name_and_domain(resource[:tenant])"},{"line_number":46,"context_line":"      set_project(tenant_name, tenant_domain)"},{"line_number":47,"context_line":"      @property_hash[:tenant] \u003d tenant_name"},{"line_number":48,"context_line":"      @property_hash[:tenant_domain] \u003d tenant_domain"},{"line_number":49,"context_line":"    end"},{"line_number":50,"context_line":"    @property_hash[:ensure] \u003d :present"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_cd94bfe3","line":47,"updated":"2015-09-28 15:31:10.000000000","message":"This should be the same as resource[:tenant]","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"23109eeb19331350a9f3e9737e5e7add50b3ce75","unresolved":false,"context_lines":[{"line_number":44,"context_line":"      # See if resource[:tenant_domain] or user specified as tenant::domain"},{"line_number":45,"context_line":"      tenant_name, tenant_domain \u003d self.class.name_and_domain(resource[:tenant])"},{"line_number":46,"context_line":"      set_project(tenant_name, tenant_domain)"},{"line_number":47,"context_line":"      @property_hash[:tenant] \u003d tenant_name"},{"line_number":48,"context_line":"      @property_hash[:tenant_domain] \u003d tenant_domain"},{"line_number":49,"context_line":"    end"},{"line_number":50,"context_line":"    @property_hash[:ensure] \u003d :present"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_02b81d29","line":47,"in_reply_to":"9a1a9d01_5cf04967","updated":"2015-09-29 12:10:13.000000000","message":"Sorry for the confusion, when I said obsolete I meant, in following patch sets I restored the original code.\n\nActually the whole deprecated section to disappear soon. So we shouldn\u0027t spent too much energy onto it. Of course besides the autorequire matter which impact the rest anyway.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"b36e8ddc4d99fcf4a0ee07a0ae1f00c9d2b13904","unresolved":false,"context_lines":[{"line_number":44,"context_line":"      # See if resource[:tenant_domain] or user specified as tenant::domain"},{"line_number":45,"context_line":"      tenant_name, tenant_domain \u003d self.class.name_and_domain(resource[:tenant])"},{"line_number":46,"context_line":"      set_project(tenant_name, tenant_domain)"},{"line_number":47,"context_line":"      @property_hash[:tenant] \u003d tenant_name"},{"line_number":48,"context_line":"      @property_hash[:tenant_domain] \u003d tenant_domain"},{"line_number":49,"context_line":"    end"},{"line_number":50,"context_line":"    @property_hash[:ensure] \u003d :present"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_dc43f940","line":47,"in_reply_to":"9a1a9d01_cd94bfe3","updated":"2015-09-29 01:03:45.000000000","message":"This is obsolete.\n\nI used tenant_domain parameter which is not possible because autorequire takes only a name (unless more advanced catalog parsing is done and I\u0027m still not sure that would work)","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d640904da60aa407d5db8bfbcd6661117544cf2f","unresolved":false,"context_lines":[{"line_number":44,"context_line":"      # See if resource[:tenant_domain] or user specified as tenant::domain"},{"line_number":45,"context_line":"      tenant_name, tenant_domain \u003d self.class.name_and_domain(resource[:tenant])"},{"line_number":46,"context_line":"      set_project(tenant_name, tenant_domain)"},{"line_number":47,"context_line":"      @property_hash[:tenant] \u003d tenant_name"},{"line_number":48,"context_line":"      @property_hash[:tenant_domain] \u003d tenant_domain"},{"line_number":49,"context_line":"    end"},{"line_number":50,"context_line":"    @property_hash[:ensure] \u003d :present"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_5cf04967","line":47,"in_reply_to":"9a1a9d01_dc43f940","updated":"2015-09-29 01:40:23.000000000","message":"But do you still allow the user to specify the tenant like this::\n\n   keystone_tenant { \u0027projectname::projectdomain\u0027: ...}\n\n   keystone_user { \u0027someuser\u0027:\n     tenant \u003d\u003e \u0027projectname::projectdomain\u0027\n   }\n\nIf so, then resource[:tenant] is \u0027projectname::projectdomain\u0027, and @property_hash[:tenant] should also be \u0027projectname::projectdomain\u0027.\n\nautorequire takes the name of the resource in the catalog, which in this case is \u0027projectname::projectdomain\u0027\n\nIf Sofer is able to get the more advanced catalog parsing working, then we can revisit this.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"dcc1efb3c32ee1aedfa13ef6919f0deda867f553","unresolved":false,"context_lines":[{"line_number":204,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":205,"context_line":"  # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":206,"context_line":"  def tenant\u003d(value)"},{"line_number":207,"context_line":"    @property_hash[:tenant] \u003d set_project(value, nil)"},{"line_number":208,"context_line":"  end"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_0d607700","line":207,"updated":"2015-09-28 15:31:10.000000000","message":"@property_hash[:tenant] should be the same as resource[:tenant]","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"b36e8ddc4d99fcf4a0ee07a0ae1f00c9d2b13904","unresolved":false,"context_lines":[{"line_number":204,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":205,"context_line":"  # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":206,"context_line":"  def tenant\u003d(value)"},{"line_number":207,"context_line":"    @property_hash[:tenant] \u003d set_project(value, nil)"},{"line_number":208,"context_line":"  end"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_fc959d86","line":207,"in_reply_to":"9a1a9d01_0d607700","updated":"2015-09-29 01:03:45.000000000","message":"Same as above.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"23109eeb19331350a9f3e9737e5e7add50b3ce75","unresolved":false,"context_lines":[{"line_number":204,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":205,"context_line":"  # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":206,"context_line":"  def tenant\u003d(value)"},{"line_number":207,"context_line":"    @property_hash[:tenant] \u003d set_project(value, nil)"},{"line_number":208,"context_line":"  end"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_c2eb0512","line":207,"in_reply_to":"9a1a9d01_9cfa6142","updated":"2015-09-29 12:10:13.000000000","message":"Same","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d640904da60aa407d5db8bfbcd6661117544cf2f","unresolved":false,"context_lines":[{"line_number":204,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":205,"context_line":"  # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":206,"context_line":"  def tenant\u003d(value)"},{"line_number":207,"context_line":"    @property_hash[:tenant] \u003d set_project(value, nil)"},{"line_number":208,"context_line":"  end"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"  # DEPRECATED - To be removed in next release (Liberty)"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_9cfa6142","line":207,"in_reply_to":"9a1a9d01_fc959d86","updated":"2015-09-29 01:40:23.000000000","message":"see above","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"9dd2efeab2a5b9d6c12768a768e2a239b49280ae","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    if resource[:tenant]"},{"line_number":41,"context_line":"      # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":42,"context_line":"      # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":43,"context_line":"      @property_hash[:tenant] \u003d set_project(resource[:tenant])"},{"line_number":44,"context_line":"    end"},{"line_number":45,"context_line":"    @property_hash[:ensure] \u003d :present"},{"line_number":46,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":11,"id":"9a1a9d01_45560d5d","line":43,"updated":"2015-09-29 17:35:40.000000000","message":"I think you should rebase this on top of https://review.openstack.org/#/c/228744/ so we can get rid of this tenant crap.","commit_id":"acc2c601e745be31a87d8f6282f46cc42f83b16c"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"66069e97b4c6fd5349838f583eb296812d16d6c3","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    if resource[:tenant]"},{"line_number":41,"context_line":"      # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":42,"context_line":"      # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":43,"context_line":"      @property_hash[:tenant] \u003d set_project(resource[:tenant])"},{"line_number":44,"context_line":"    end"},{"line_number":45,"context_line":"    @property_hash[:ensure] \u003d :present"},{"line_number":46,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":11,"id":"9a1a9d01_818bab67","line":43,"in_reply_to":"9a1a9d01_45560d5d","updated":"2015-09-29 23:38:15.000000000","message":"I can\u0027t agree more. The reason I\u0027ve been doing it the other way is to be able to back-port to stable/kilo. \n\nThis one, besides the nil/raise issue, can make it, this way we can have a better road for a back-port if it eventually happens to be needed.","commit_id":"acc2c601e745be31a87d8f6282f46cc42f83b16c"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d9dfaff44cf8103170a89ba015bd2b103a0296b0","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    if resource[:tenant]"},{"line_number":41,"context_line":"      # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":42,"context_line":"      # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":43,"context_line":"      @property_hash[:tenant] \u003d set_project(resource[:tenant])"},{"line_number":44,"context_line":"    end"},{"line_number":45,"context_line":"    @property_hash[:ensure] \u003d :present"},{"line_number":46,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":11,"id":"9a1a9d01_92d3f703","line":43,"in_reply_to":"9a1a9d01_818bab67","updated":"2015-09-30 02:08:22.000000000","message":"Ah, that\u0027s right.  We have to fix the indirection performance in kilo so we have to support tenant.\n\nThen for kilo, @property_hash[:tenant] \u003d resource[:tenant]","commit_id":"acc2c601e745be31a87d8f6282f46cc42f83b16c"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"4bee4634bd6c52522396c276f379b0e0f183e980","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    if resource[:tenant]"},{"line_number":41,"context_line":"      # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":42,"context_line":"      # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":43,"context_line":"      @property_hash[:tenant] \u003d set_project(resource[:tenant])"},{"line_number":44,"context_line":"    end"},{"line_number":45,"context_line":"    @property_hash[:ensure] \u003d :present"},{"line_number":46,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":11,"id":"9a1a9d01_32c30bef","line":43,"in_reply_to":"9a1a9d01_92d3f703","updated":"2015-09-30 02:27:26.000000000","message":"Done","commit_id":"acc2c601e745be31a87d8f6282f46cc42f83b16c"}],"lib/puppet/provider/keystone_user_role/openstack.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"4bf33712f1c3bfc753fc8d4472fcf62f2ecad4f5","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    @domain"},{"line_number":120,"context_line":"  end"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"  def get_user_id"},{"line_number":123,"context_line":"    user_name, domain_name \u003d Util.split_domain(get_user)"},{"line_number":124,"context_line":"    user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":125,"context_line":"    user_id \u003d user ? user[:id] : nil"}],"source_content_type":"text/x-ruby","patch_set":5,"id":"ba15a1d1_76b73085","line":122,"updated":"2015-09-24 10:30:56.000000000","message":"For efficiency (not calling fetch_user) you could start with\n  return @user_id if defined?(@user_id)\n\nas you do in get_project_id.\n\nand change the last line with\n  @user_id \u003d user_id\n\nBut I may be missing some logic here.","commit_id":"ead72edfb37827ce5a632ec05ada5f5b05da8a31"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"e42b030634bda1bfabc7a35167776e701173d5a6","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    @domain"},{"line_number":120,"context_line":"  end"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"  def get_user_id"},{"line_number":123,"context_line":"    user_name, domain_name \u003d Util.split_domain(get_user)"},{"line_number":124,"context_line":"    user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":125,"context_line":"    user_id \u003d user ? user[:id] : nil"}],"source_content_type":"text/x-ruby","patch_set":5,"id":"9a1a9d01_e060c637","line":122,"in_reply_to":"ba15a1d1_76b73085","updated":"2015-09-28 01:09:11.000000000","message":"Gotcha!","commit_id":"ead72edfb37827ce5a632ec05ada5f5b05da8a31"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"4bf33712f1c3bfc753fc8d4472fcf62f2ecad4f5","unresolved":false,"context_lines":[{"line_number":135,"context_line":"    else"},{"line_number":136,"context_line":"      project \u003d self.class.fetch_project(project_name, domain_name)"},{"line_number":137,"context_line":"      project_id \u003d project ? project[:id] : nil"},{"line_number":138,"context_line":"      @project_id ||\u003d project_id"},{"line_number":139,"context_line":"    end"},{"line_number":140,"context_line":"    @project_id"},{"line_number":141,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":5,"id":"ba15a1d1_16b35499","line":138,"updated":"2015-09-24 10:30:56.000000000","message":"If you went that far, the ||\u003d looks like it\u0027s not needed anymore.","commit_id":"ead72edfb37827ce5a632ec05ada5f5b05da8a31"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"e42b030634bda1bfabc7a35167776e701173d5a6","unresolved":false,"context_lines":[{"line_number":135,"context_line":"    else"},{"line_number":136,"context_line":"      project \u003d self.class.fetch_project(project_name, domain_name)"},{"line_number":137,"context_line":"      project_id \u003d project ? project[:id] : nil"},{"line_number":138,"context_line":"      @project_id ||\u003d project_id"},{"line_number":139,"context_line":"    end"},{"line_number":140,"context_line":"    @project_id"},{"line_number":141,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":5,"id":"9a1a9d01_80222a00","line":138,"in_reply_to":"ba15a1d1_16b35499","updated":"2015-09-28 01:09:11.000000000","message":"Done","commit_id":"ead72edfb37827ce5a632ec05ada5f5b05da8a31"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"dcc1efb3c32ee1aedfa13ef6919f0deda867f553","unresolved":false,"context_lines":[{"line_number":128,"context_line":"      user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":129,"context_line":"      @user_id \u003d user ? user[:id] : nil"},{"line_number":130,"context_line":"    end"},{"line_number":131,"context_line":"    @user_id"},{"line_number":132,"context_line":"  end"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"  def get_project_id"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_103c0cc7","line":131,"updated":"2015-09-28 15:31:10.000000000","message":"I think this can be simplified.  There should never be a case where user_name is nil.  The user is required by keystone_user_role.  Project (below) is different because project is not required - a user can have a role on a project or on a domain.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"b36e8ddc4d99fcf4a0ee07a0ae1f00c9d2b13904","unresolved":false,"context_lines":[{"line_number":128,"context_line":"      user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":129,"context_line":"      @user_id \u003d user ? user[:id] : nil"},{"line_number":130,"context_line":"    end"},{"line_number":131,"context_line":"    @user_id"},{"line_number":132,"context_line":"  end"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"  def get_project_id"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_1c5651ce","line":131,"in_reply_to":"9a1a9d01_103c0cc7","updated":"2015-09-29 01:03:45.000000000","message":"In theory you\u0027re right but the reality has shown otherwise.\n\nPart of the issue resides in the fact that autorequire cannot always guarantee the existence of the user, unless we use full domain names. Therefore having a fail safe won\u0027t hurt.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d640904da60aa407d5db8bfbcd6661117544cf2f","unresolved":false,"context_lines":[{"line_number":128,"context_line":"      user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":129,"context_line":"      @user_id \u003d user ? user[:id] : nil"},{"line_number":130,"context_line":"    end"},{"line_number":131,"context_line":"    @user_id"},{"line_number":132,"context_line":"  end"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"  def get_project_id"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_f0b14148","line":131,"in_reply_to":"9a1a9d01_1c5651ce","updated":"2015-09-29 01:40:23.000000000","message":"\u003e In theory you\u0027re right but the reality has shown otherwise.\n\nHow so?\n\nIf the author of the manifest specified a user that does not have a corresponding keystone_user declaration, we should just thrown an exception, or otherwise handle it as an error condition the way puppet handles other cases where a resource is attempted to be used without being declared.\n\nautorequire can always guarantee the existence of the user without the use of full domain names.  The thing returned by autorequire is the title of the resource.  That can be a single name (if it is in the default domain) or a compound name with \"::domain\".","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"51ab9b7458674b38857ebee9fbc569c98c4a6a33","unresolved":false,"context_lines":[{"line_number":128,"context_line":"      user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":129,"context_line":"      @user_id \u003d user ? user[:id] : nil"},{"line_number":130,"context_line":"    end"},{"line_number":131,"context_line":"    @user_id"},{"line_number":132,"context_line":"  end"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"  def get_project_id"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_61c21736","line":131,"in_reply_to":"9a1a9d01_653eb129","updated":"2015-09-29 23:42:21.000000000","message":"Will do for the exception.\n\nLooking at the autorequire\u0027s investigation work. Thanks","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"23109eeb19331350a9f3e9737e5e7add50b3ce75","unresolved":false,"context_lines":[{"line_number":128,"context_line":"      user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":129,"context_line":"      @user_id \u003d user ? user[:id] : nil"},{"line_number":130,"context_line":"    end"},{"line_number":131,"context_line":"    @user_id"},{"line_number":132,"context_line":"  end"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"  def get_project_id"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_f8f03693","line":131,"in_reply_to":"9a1a9d01_f0b14148","updated":"2015-09-29 12:10:13.000000000","message":"I\u0027ve a good practical example I\u0027ve bumped into:\n\nThe ::keystone::resource::service_identity definition manifest:\nIf the name doesn\u0027t contain the domain (and when the domain is different from the default) then one hits the case where the user is not found. Even if the $user_domain parameter is set. The reason is the manifest doesn\u0027t use the $user_domain for keystone_user_role.\n\nAlthough I tend to agree with raising an error in such case, so far this has been about keeping a balance between backward compatibility and moving on. I\u0027ll look at raising an exception if #fetch_user returns nil.\n\nI disagree with your comment about autorequire, because autorequire must make sure the exact user::domain mentioned within the catalog has been treated before, not just the same user name from some domain.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"63fd1b9e13d8b693bdea62a04e287f7d8bc90bf0","unresolved":false,"context_lines":[{"line_number":128,"context_line":"      user \u003d self.class.fetch_user(user_name, domain_name)"},{"line_number":129,"context_line":"      @user_id \u003d user ? user[:id] : nil"},{"line_number":130,"context_line":"    end"},{"line_number":131,"context_line":"    @user_id"},{"line_number":132,"context_line":"  end"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"  def get_project_id"}],"source_content_type":"text/x-ruby","patch_set":8,"id":"9a1a9d01_653eb129","line":131,"in_reply_to":"9a1a9d01_f8f03693","updated":"2015-09-29 17:35:35.000000000","message":"\u003e The ::keystone::resource::service_identity definition manifest: If the name doesn\u0027t contain the domain (and when the domain is different from the default) then one hits the case where the user is not found. Even if the $user_domain parameter is set. The reason is the manifest doesn\u0027t use the $user_domain for keystone_user_role.\n\nYeah, that\u0027s bad, and that is a limitation of keystone_user_role.  I think we are going to have to add username, project_name, user_domain_name, project_domain_name, and domain_name (for domain scoped roles) parameters to keystone_user_role.\n\nIn the case you describe, we should just raise an exception, or whatever puppet does in the case where you attempt to use a resource name that is not defined.  It doesn\u0027t preserve backwards compatibility to return nil here.\n\nautorequire will work, but it has to return the title of the resource, and manifest writers have to be consistent with the usage.  See https://review.openstack.org/#/c/226919/ comment 3.","commit_id":"c60bfb244f1a4be1d19c2031e024503e3a03bd20"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"9dd2efeab2a5b9d6c12768a768e2a239b49280ae","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    elsif get_domain"},{"line_number":92,"context_line":"      properties \u003c\u003c \u0027--domain\u0027 \u003c\u003c get_domain"},{"line_number":93,"context_line":"    else"},{"line_number":94,"context_line":"      raise(Puppet::Error, \u0027No project or domain specified for role\u0027)"},{"line_number":95,"context_line":"    end"},{"line_number":96,"context_line":"    properties \u003c\u003c \u0027--user\u0027 \u003c\u003c get_user_id"},{"line_number":97,"context_line":"    properties"}],"source_content_type":"text/x-ruby","patch_set":11,"id":"9a1a9d01_a5d369d4","line":94,"updated":"2015-09-29 17:35:40.000000000","message":"The method name is `err` not `error`.  I think the convention is to use the `err` method in cases like this, instead of directly raising an exception.","commit_id":"acc2c601e745be31a87d8f6282f46cc42f83b16c"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d9dfaff44cf8103170a89ba015bd2b103a0296b0","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    elsif get_domain"},{"line_number":92,"context_line":"      properties \u003c\u003c \u0027--domain\u0027 \u003c\u003c get_domain"},{"line_number":93,"context_line":"    else"},{"line_number":94,"context_line":"      raise(Puppet::Error, \u0027No project or domain specified for role\u0027)"},{"line_number":95,"context_line":"    end"},{"line_number":96,"context_line":"    properties \u003c\u003c \u0027--user\u0027 \u003c\u003c get_user_id"},{"line_number":97,"context_line":"    properties"}],"source_content_type":"text/x-ruby","patch_set":11,"id":"9a1a9d01_12060786","line":94,"in_reply_to":"9a1a9d01_a1fb4fc0","updated":"2015-09-30 02:08:22.000000000","message":"It doesn\u0027t matter - use an exception or err, whatever.","commit_id":"acc2c601e745be31a87d8f6282f46cc42f83b16c"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"66069e97b4c6fd5349838f583eb296812d16d6c3","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    elsif get_domain"},{"line_number":92,"context_line":"      properties \u003c\u003c \u0027--domain\u0027 \u003c\u003c get_domain"},{"line_number":93,"context_line":"    else"},{"line_number":94,"context_line":"      raise(Puppet::Error, \u0027No project or domain specified for role\u0027)"},{"line_number":95,"context_line":"    end"},{"line_number":96,"context_line":"    properties \u003c\u003c \u0027--user\u0027 \u003c\u003c get_user_id"},{"line_number":97,"context_line":"    properties"}],"source_content_type":"text/x-ruby","patch_set":11,"id":"9a1a9d01_a1fb4fc0","line":94,"in_reply_to":"9a1a9d01_a5d369d4","updated":"2015-09-29 23:38:15.000000000","message":"I understand the legacy \u0027error\u0027 call should have been \u0027err\u0027 and I\u0027d love to learn more about such convention (a link please?). In the meantime since we\u0027ve not been using \u0027err\u0027 much and to be consistent I used raise. Maybe, in the light of such convention, we could revisit, in a specific patch, the different \u0027raise\u0027 calls and ventilates them to \u0027err\u0027 calls and so.","commit_id":"acc2c601e745be31a87d8f6282f46cc42f83b16c"}],"lib/puppet/type/keystone_user.rb":[{"author":{"_account_id":3153,"name":"Emilien Macchi","email":"emilien@redhat.com","username":"emilienm"},"change_message_id":"47551788b7f7745f670a6626e8b00f586d2e835b","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    end"},{"line_number":90,"context_line":"  end"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"  # we should not do anything until the keystone service is started"},{"line_number":93,"context_line":"  autorequire(:anchor) do"},{"line_number":94,"context_line":"    [\u0027keystone_started\u0027,\u0027default_domain_created\u0027]"},{"line_number":95,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":13,"id":"9a1a9d01_be1ed409","line":92,"updated":"2015-09-30 23:56:13.000000000","message":"I think you might want to move that to https://review.openstack.org/#/c/228744 isn\u0027t?\nYou\u0027re mixing both patches iiuc.","commit_id":"07b08a13be0fbef16d090db9fb74e5646b887262"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"12f228bc00306ca4b3562e548f97169d9f2f17d4","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    end"},{"line_number":90,"context_line":"  end"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"  # we should not do anything until the keystone service is started"},{"line_number":93,"context_line":"  autorequire(:anchor) do"},{"line_number":94,"context_line":"    [\u0027keystone_started\u0027,\u0027default_domain_created\u0027]"},{"line_number":95,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":13,"id":"9a1a9d01_1e07c082","line":92,"in_reply_to":"9a1a9d01_be1ed409","updated":"2015-10-01 00:03:56.000000000","message":"Yeah nothing changed here, the reason the order of those method is different is because something was added then removed.\nI\u0027ll restore that.\nThanks.","commit_id":"07b08a13be0fbef16d090db9fb74e5646b887262"},{"author":{"_account_id":3153,"name":"Emilien Macchi","email":"emilien@redhat.com","username":"emilienm"},"change_message_id":"47551788b7f7745f670a6626e8b00f586d2e835b","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    self[:domain] or Util.split_domain(self[:name])[1]"},{"line_number":100,"context_line":"  end"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"  autorequire(:keystone_tenant) do"},{"line_number":103,"context_line":"    # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":104,"context_line":"    # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":105,"context_line":"    self[:tenant]"}],"source_content_type":"text/x-ruby","patch_set":13,"id":"9a1a9d01_fede8c3c","line":102,"updated":"2015-09-30 23:56:13.000000000","message":"ditto.","commit_id":"07b08a13be0fbef16d090db9fb74e5646b887262"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"12f228bc00306ca4b3562e548f97169d9f2f17d4","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    self[:domain] or Util.split_domain(self[:name])[1]"},{"line_number":100,"context_line":"  end"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"  autorequire(:keystone_tenant) do"},{"line_number":103,"context_line":"    # DEPRECATED - To be removed in next release (Liberty)"},{"line_number":104,"context_line":"    # https://bugs.launchpad.net/puppet-keystone/+bug/1472437"},{"line_number":105,"context_line":"    self[:tenant]"}],"source_content_type":"text/x-ruby","patch_set":13,"id":"9a1a9d01_feff6c63","line":102,"in_reply_to":"9a1a9d01_fede8c3c","updated":"2015-10-01 00:03:56.000000000","message":"Same as above","commit_id":"07b08a13be0fbef16d090db9fb74e5646b887262"}]}