)]}'
{"manifests/bucket.pp":[{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"9da92fde12914904554d41c3f67ff1b730ca3113","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"define ceph::bucket ("},{"line_number":59,"context_line":"  $ensure      \u003d \u0027present\u0027,"},{"line_number":60,"context_line":"  $type        \u003d undef,"},{"line_number":61,"context_line":"  $parent_type \u003d \u0027root\u0027,"},{"line_number":62,"context_line":"  $parent_name \u003d \u0027default\u0027,"},{"line_number":63,"context_line":"){"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_eeb6f2d9","line":60,"range":{"start_line":60,"start_character":17,"end_line":60,"end_character":22},"updated":"2017-05-18 19:57:03.000000000","message":"is this required if ensure \u003d\u003d \u0027present\u0027? It seems that this would be problematic if not required for the add-bucket command","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":19663,"name":"Tom Verdaat","email":"tom@server.biz","username":"toMeloos"},"change_message_id":"b100cee26db2ce220aa22be320386e73c832ba58","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"define ceph::bucket ("},{"line_number":59,"context_line":"  $ensure      \u003d \u0027present\u0027,"},{"line_number":60,"context_line":"  $type        \u003d undef,"},{"line_number":61,"context_line":"  $parent_type \u003d \u0027root\u0027,"},{"line_number":62,"context_line":"  $parent_name \u003d \u0027default\u0027,"},{"line_number":63,"context_line":"){"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_b6295a9c","line":60,"range":{"start_line":60,"start_character":17,"end_line":60,"end_character":22},"in_reply_to":"ff0f0b1f_eeb6f2d9","updated":"2017-05-21 20:58:12.000000000","message":"Yes, but it is not required if ensure \u003d\u003d \u0027absent\u0027. Hence the undef.","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"9da92fde12914904554d41c3f67ff1b730ca3113","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    Ceph::Mon \u003c||\u003e -\u003e Exec[\"create-${name}\"]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    exec { \"create-${name}\":"},{"line_number":70,"context_line":"      command \u003d\u003e \"ceph osd crush add-bucket ${name} ${type}\","},{"line_number":71,"context_line":"      unless  \u003d\u003e \"ceph osd crush dump | grep \u0027name\u0027 | grep -q \u0027\\\"${name}\\\"\u0027\","},{"line_number":72,"context_line":"    }"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_ce7d6e1d","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":26},"updated":"2017-05-18 19:57:03.000000000","message":"So i think this needs to be \"create-bucket-${name}\" because it\u0027ll have conflicts if there is already a \"create-${name}\" somewhere. For these create, move, delete exec names they should include something like -bucket- so that they will be less likely to conflict with something else","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":19663,"name":"Tom Verdaat","email":"tom@server.biz","username":"toMeloos"},"change_message_id":"b100cee26db2ce220aa22be320386e73c832ba58","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    Ceph::Mon \u003c||\u003e -\u003e Exec[\"create-${name}\"]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    exec { \"create-${name}\":"},{"line_number":70,"context_line":"      command \u003d\u003e \"ceph osd crush add-bucket ${name} ${type}\","},{"line_number":71,"context_line":"      unless  \u003d\u003e \"ceph osd crush dump | grep \u0027name\u0027 | grep -q \u0027\\\"${name}\\\"\u0027\","},{"line_number":72,"context_line":"    }"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_b654ba22","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":26},"in_reply_to":"ff0f0b1f_ce7d6e1d","updated":"2017-05-21 20:58:12.000000000","message":"Done","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"9da92fde12914904554d41c3f67ff1b730ca3113","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    if ($parent_name) and ($parent_type) {"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"      if (defined(ceph::bucket[$parent_name])) {"},{"line_number":77,"context_line":"        Ceph::Bucket[$parent_name] -\u003e Exec[\"move-${name}\"]"},{"line_number":78,"context_line":"      }"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_ce768ed9","line":76,"range":{"start_line":76,"start_character":18,"end_line":76,"end_character":30},"updated":"2017-05-18 19:57:03.000000000","message":"I think you should replace this with the spaceship operator (see next comment) but this would be defined(Ceph::Bucket[$parent_name]) if you did it this way.","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":19663,"name":"Tom Verdaat","email":"tom@server.biz","username":"toMeloos"},"change_message_id":"b100cee26db2ce220aa22be320386e73c832ba58","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    if ($parent_name) and ($parent_type) {"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"      if (defined(ceph::bucket[$parent_name])) {"},{"line_number":77,"context_line":"        Ceph::Bucket[$parent_name] -\u003e Exec[\"move-${name}\"]"},{"line_number":78,"context_line":"      }"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_16600601","line":76,"range":{"start_line":76,"start_character":18,"end_line":76,"end_character":30},"in_reply_to":"ff0f0b1f_ce768ed9","updated":"2017-05-21 20:58:12.000000000","message":"Done","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"9da92fde12914904554d41c3f67ff1b730ca3113","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    if ($parent_name) and ($parent_type) {"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"      if (defined(ceph::bucket[$parent_name])) {"},{"line_number":77,"context_line":"        Ceph::Bucket[$parent_name] -\u003e Exec[\"move-${name}\"]"},{"line_number":78,"context_line":"      }"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"      exec { \"move-${name}\":"},{"line_number":81,"context_line":"        command     \u003d\u003e \"ceph osd crush move ${name} ${parent_type}\u003d${parent_name}\","}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_ce27ae12","line":78,"range":{"start_line":76,"start_character":0,"end_line":78,"end_character":7},"updated":"2017-05-18 19:57:03.000000000","message":"So an alternative to this would be to use the spaceship operator instead so that it wouldn\u0027t have to be a hard requirement to have the parent included in the same catalog. Additionally it would do away with the defined function which can be problematic due to catalog compilation\n\n\nI\u0027d replace this block with:\n\n  Ceph::Bucket\u003c| title \u003d\u003d $parent_name |\u003e -\u003e Exec[\"move-${name}\"]\n\n\nBut note that this block should be below the exec { \"move-${name}\": ...} bit","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":19663,"name":"Tom Verdaat","email":"tom@server.biz","username":"toMeloos"},"change_message_id":"b100cee26db2ce220aa22be320386e73c832ba58","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    if ($parent_name) and ($parent_type) {"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"      if (defined(ceph::bucket[$parent_name])) {"},{"line_number":77,"context_line":"        Ceph::Bucket[$parent_name] -\u003e Exec[\"move-${name}\"]"},{"line_number":78,"context_line":"      }"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"      exec { \"move-${name}\":"},{"line_number":81,"context_line":"        command     \u003d\u003e \"ceph osd crush move ${name} ${parent_type}\u003d${parent_name}\","}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_3649ca8a","line":78,"range":{"start_line":76,"start_character":0,"end_line":78,"end_character":7},"in_reply_to":"ff0f0b1f_ce27ae12","updated":"2017-05-21 20:58:12.000000000","message":"Done","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"}],"manifests/cache.pp":[{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"a1f8082a8b93e8890c04bcbde2c4e41f04430c60","unresolved":false,"context_lines":[{"line_number":27,"context_line":"#  Mandatory."},{"line_number":28,"context_line":"#"},{"line_number":29,"context_line":"# [*cache_mode*] type of caching to apply."},{"line_number":30,"context_line":"#  Notice: Switching from writeback to readonly can result in loss of data! Since a writeback cache may have modified data,"},{"line_number":31,"context_line":"#  you must take steps to ensure that you do not lose any recent changes to objects in the cache before you disable and remove it."},{"line_number":32,"context_line":"#  See http://ceph.com/docs/master/rados/operations/cache-tiering/"},{"line_number":33,"context_line":"#  Optional. String (\u0027none\u0027, \u0027readonly\u0027, \u0027forward\u0027 or \u0027writeback\u0027)"},{"line_number":34,"context_line":"#  Defaults to \u0027readonly\u0027."},{"line_number":35,"context_line":"#"},{"line_number":36,"context_line":"# [*delete_cache*] if the given pool should be removed as a caching tier."}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_989ea361","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":66},"updated":"2017-05-18 17:02:48.000000000","message":"Just as a heads up, we usually wrap these to 80 chars for readability","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"f09c71d7d5574319cdc2ef01d58665b4066f6928","unresolved":false,"context_lines":[{"line_number":27,"context_line":"#  Mandatory."},{"line_number":28,"context_line":"#"},{"line_number":29,"context_line":"# [*cache_mode*] type of caching to apply."},{"line_number":30,"context_line":"#  Notice: Switching from writeback to readonly can result in loss of data! Since a writeback cache may have modified data,"},{"line_number":31,"context_line":"#  you must take steps to ensure that you do not lose any recent changes to objects in the cache before you disable and remove it."},{"line_number":32,"context_line":"#  See http://ceph.com/docs/master/rados/operations/cache-tiering/"},{"line_number":33,"context_line":"#  Optional. String (\u0027none\u0027, \u0027readonly\u0027, \u0027forward\u0027 or \u0027writeback\u0027)"},{"line_number":34,"context_line":"#  Defaults to \u0027readonly\u0027."},{"line_number":35,"context_line":"#"},{"line_number":36,"context_line":"# [*delete_cache*] if the given pool should be removed as a caching tier."}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_f8eebf86","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":66},"in_reply_to":"ff0f0b1f_3819d7d6","updated":"2017-05-18 17:10:01.000000000","message":"Yea we\u0027re less strict on the actual puppet. I was referring to just these comments because in gerrit they get cut off.  Generally we prefer 80 when possible but allow 140 as needed.","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":19663,"name":"Tom Verdaat","email":"tom@server.biz","username":"toMeloos"},"change_message_id":"10edb1a1446f07b8131076d78299082a8b81a92f","unresolved":false,"context_lines":[{"line_number":27,"context_line":"#  Mandatory."},{"line_number":28,"context_line":"#"},{"line_number":29,"context_line":"# [*cache_mode*] type of caching to apply."},{"line_number":30,"context_line":"#  Notice: Switching from writeback to readonly can result in loss of data! Since a writeback cache may have modified data,"},{"line_number":31,"context_line":"#  you must take steps to ensure that you do not lose any recent changes to objects in the cache before you disable and remove it."},{"line_number":32,"context_line":"#  See http://ceph.com/docs/master/rados/operations/cache-tiering/"},{"line_number":33,"context_line":"#  Optional. String (\u0027none\u0027, \u0027readonly\u0027, \u0027forward\u0027 or \u0027writeback\u0027)"},{"line_number":34,"context_line":"#  Defaults to \u0027readonly\u0027."},{"line_number":35,"context_line":"#"},{"line_number":36,"context_line":"# [*delete_cache*] if the given pool should be removed as a caching tier."}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_3819d7d6","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":66},"in_reply_to":"ff0f0b1f_989ea361","updated":"2017-05-18 17:07:33.000000000","message":"I\u0027ll fix that in the next set! The new puppet guideline as enforced by lint is 140 though.","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"}],"manifests/pool.pp":[{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"f3078d186db682c42cc442ae3d98f80e3e16b285","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    exec { \"create-${name}\":"},{"line_number":92,"context_line":"      command \u003d\u003e \"/bin/true # comment to satisfy puppet syntax requirements"},{"line_number":93,"context_line":"set -ex"},{"line_number":94,"context_line":"ceph osd pool create ${name} ${pg_num} ${pgp_num} ${type} ${ruleset}\","},{"line_number":95,"context_line":"      unless  \u003d\u003e \"/bin/true # comment to satisfy puppet syntax requirements"},{"line_number":96,"context_line":"set -ex"},{"line_number":97,"context_line":"ceph osd lspools | grep \u0027 ${name},\u0027\","}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_6ee30235","line":94,"updated":"2017-05-18 19:28:24.000000000","message":"So as I suspected, this bit needs to get updated in the unit tests\nhttp://logs.openstack.org/50/466050/1/check/gate-puppet-ceph-puppet-unit-4.5-centos-7/c93cb91/console.html#_2017-05-18_17_01_03_102605\n\n\nYou\u0027ll need to update https://github.com/openstack/puppet-ceph/blob/master/spec/defines/ceph_pool_spec.rb#L41","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":19663,"name":"Tom Verdaat","email":"tom@server.biz","username":"toMeloos"},"change_message_id":"b86404c3787943bb31b32573c4c378051e74cf94","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    exec { \"create-${name}\":"},{"line_number":92,"context_line":"      command \u003d\u003e \"/bin/true # comment to satisfy puppet syntax requirements"},{"line_number":93,"context_line":"set -ex"},{"line_number":94,"context_line":"ceph osd pool create ${name} ${pg_num} ${pgp_num} ${type} ${ruleset}\","},{"line_number":95,"context_line":"      unless  \u003d\u003e \"/bin/true # comment to satisfy puppet syntax requirements"},{"line_number":96,"context_line":"set -ex"},{"line_number":97,"context_line":"ceph osd lspools | grep \u0027 ${name},\u0027\","}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_55ef74af","line":94,"in_reply_to":"ff0f0b1f_6ee30235","updated":"2017-05-21 21:46:55.000000000","message":"Done","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"f3078d186db682c42cc442ae3d98f80e3e16b285","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        require \u003d\u003e Exec[\"create-${name}\"],"},{"line_number":147,"context_line":"        timeout \u003d\u003e $exec_timeout,"},{"line_number":148,"context_line":"      }"},{"line_number":149,"context_line":"    }"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"  } elsif $ensure \u003d\u003d absent {"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_cec56e90","line":149,"updated":"2017-05-18 19:28:24.000000000","message":"Additionally you should add a test case in for this as well\n\nhttps://github.com/openstack/puppet-ceph/blob/master/spec/defines/ceph_pool_spec.rb#L24-L55","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":19663,"name":"Tom Verdaat","email":"tom@server.biz","username":"toMeloos"},"change_message_id":"b86404c3787943bb31b32573c4c378051e74cf94","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        require \u003d\u003e Exec[\"create-${name}\"],"},{"line_number":147,"context_line":"        timeout \u003d\u003e $exec_timeout,"},{"line_number":148,"context_line":"      }"},{"line_number":149,"context_line":"    }"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"  } elsif $ensure \u003d\u003d absent {"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-puppet","patch_set":1,"id":"ff0f0b1f_352c0001","line":149,"in_reply_to":"ff0f0b1f_cec56e90","updated":"2017-05-21 21:46:55.000000000","message":"Done","commit_id":"6ac8860b8414a708a2d52ef84000c0f5eb98d244"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"9fd642902bf695ce4e69b684fcb8dc051deb3cd4","unresolved":false,"context_lines":[{"line_number":82,"context_line":"    Ceph::Rule\u003c||\u003e -\u003e Exec[\"create-${name}\"]"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    if $erasure_code_profile {"},{"line_number":85,"context_line":"      Exec[\"create-${erasure_code_profile}\"] -\u003e Exec[\"create-${name}\"]"},{"line_number":86,"context_line":"      $type \u003d \"erasure ${erasure_code_profile}\""},{"line_number":87,"context_line":"    } else {"},{"line_number":88,"context_line":"      $type \u003d \u0027replicated\u0027"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"ff0f0b1f_de34b595","line":85,"range":{"start_line":85,"start_character":0,"end_line":85,"end_character":70},"updated":"2017-05-22 13:06:36.000000000","message":"This is defined on line 91 so this needs to be below that block.  As I mentioned in IRC, you may need to switch Exec[\"create-${erasure_code_profile}\"] to Exec\u003c| title \u003d\u003d \"create-${erasure_code_profile}\" |\u003e since that may not be defined before this class is called.","commit_id":"da6843c63e0726cdd3e1745a515dfbe28f296160"}]}