)]}'
{"manifests/backend/gpfs.pp":[{"author":{"_account_id":7745,"name":"Denis Egorenko","email":"degorenko@mirantis.com","username":"degorenko"},"change_message_id":"db9110506e076dde9d41b3f37924fdd914d456cf","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    \"${name}/gpfs_images_dir\":        value \u003d\u003e $gpfs_images_dir;"},{"line_number":81,"context_line":"    \"${name}/gpfs_images_share_mode\": value \u003d\u003e $gpfs_images_share_mode;"},{"line_number":82,"context_line":"    \"${name}/gpfs_max_clone_depth\":   value \u003d\u003e $gpfs_max_clone_depth;"},{"line_number":83,"context_line":"    \"${name}/gpfs_mount_point_base\":  value \u003d\u003e $gpfs_mount_point_base;"},{"line_number":84,"context_line":"    \"${name}/gpfs_sparse_volumes\":    value \u003d\u003e $gpfs_sparse_volumes;"},{"line_number":85,"context_line":"    \"${name}/gpfs_storage_pool\":      value \u003d\u003e $gpfs_storage_pool;"},{"line_number":86,"context_line":"  }"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"da20952f_a04f0a69","line":83,"updated":"2015-09-10 17:24:17.000000000","message":"\u0027gpfs_mount_point_base\u0027 variable has \u0027undef\u0027 by default and has not \u0027absent\u0027 setting, so, i think, it should have \u003cSERVICE_DEFAULT\u003e, like: https://review.openstack.org/#/c/221005","commit_id":"92de94cb417efdd9a7c57c3fb165c66a5293c2bb"},{"author":{"_account_id":7745,"name":"Denis Egorenko","email":"degorenko@mirantis.com","username":"degorenko"},"change_message_id":"f3596ebd964391425a916be150dad1dde2b1dfa1","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  if ! ($gpfs_images_share_mode in [\u0027copy\u0027, \u0027copy_on_write\u0027, \u0027\u003cSERVICE DEFAULT\u003e\u0027]) {"},{"line_number":65,"context_line":"    fail(\u0027gpfs_images_share_mode only support `copy` or `copy_on_write`\u0027)"},{"line_number":66,"context_line":"  }"},{"line_number":67,"context_line":"  if $gpfs_images_share_mode in [\u0027copy\u0027, \u0027copy_on_write\u0027] and $gpfs_images_dir \u003d\u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027 {"},{"line_number":68,"context_line":"    fail(\u0027gpfs_images_share_mode only in conjunction with gpfs_images_dir\u0027)"},{"line_number":69,"context_line":"  }"}],"source_content_type":"text/x-puppet","patch_set":4,"id":"ba15a1d1_6a0a8ed4","line":66,"updated":"2015-09-14 11:10:05.000000000","message":"I think here can be used else section, like:\n\nif ! ($gpfs_images_share_mode in [\u0027copy\u0027, \u0027copy_on_write\u0027, \u0027\u003cSERVICE DEFAULT\u003e\u0027]) {\n    fail(\u0027gpfs_images_share_mode only support `copy` or `copy_on_write`\u0027)\n } else {\n    if $gpfs_images_dir \u003d\u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027 {\n        fail(\u0027gpfs_images_share_mode only in conjunction with gpfs_images_dir\u0027)\n    }\n}\n\nWe don\u0027t need check gpfs_images_share_mode twice.","commit_id":"e391262f31b567373a2f9618b49832fba3678c7b"},{"author":{"_account_id":9212,"name":"Benedikt von St. Vieth","email":"b.von.st.vieth@fz-juelich.de","username":"bvonstvieth"},"change_message_id":"cd3c1f0830a6cd1fa5499f07fb1b27385ea871e3","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  if ! ($gpfs_images_share_mode in [\u0027copy\u0027, \u0027copy_on_write\u0027, \u0027\u003cSERVICE DEFAULT\u003e\u0027]) {"},{"line_number":65,"context_line":"    fail(\u0027gpfs_images_share_mode only support `copy` or `copy_on_write`\u0027)"},{"line_number":66,"context_line":"  }"},{"line_number":67,"context_line":"  if $gpfs_images_share_mode in [\u0027copy\u0027, \u0027copy_on_write\u0027] and $gpfs_images_dir \u003d\u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027 {"},{"line_number":68,"context_line":"    fail(\u0027gpfs_images_share_mode only in conjunction with gpfs_images_dir\u0027)"},{"line_number":69,"context_line":"  }"}],"source_content_type":"text/x-puppet","patch_set":4,"id":"ba15a1d1_bb547654","line":66,"in_reply_to":"ba15a1d1_6a0a8ed4","updated":"2015-09-14 12:34:45.000000000","message":"share_mode must be one of copy or copy_on_write or SERVICE DEFAULT.\n\nif share_mode is copy or copy_on_write, images_dir must not be SERVICE DEFAULT because it needs a path for the images.\n\nif share_mode is SERVICE DEFAULT, images_dir is allowed to be the same.\n\nsorry, its monday and i maybe have temporary problems with finding a better logic for this, but your solution does not fit? :)","commit_id":"e391262f31b567373a2f9618b49832fba3678c7b"},{"author":{"_account_id":7745,"name":"Denis Egorenko","email":"degorenko@mirantis.com","username":"degorenko"},"change_message_id":"6384bb8b168ca80d2d04fd4e566c202678a8e98e","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  if ! ($gpfs_images_share_mode in [\u0027copy\u0027, \u0027copy_on_write\u0027, \u0027\u003cSERVICE DEFAULT\u003e\u0027]) {"},{"line_number":65,"context_line":"    fail(\u0027gpfs_images_share_mode only support `copy` or `copy_on_write`\u0027)"},{"line_number":66,"context_line":"  }"},{"line_number":67,"context_line":"  if $gpfs_images_share_mode in [\u0027copy\u0027, \u0027copy_on_write\u0027] and $gpfs_images_dir \u003d\u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027 {"},{"line_number":68,"context_line":"    fail(\u0027gpfs_images_share_mode only in conjunction with gpfs_images_dir\u0027)"},{"line_number":69,"context_line":"  }"}],"source_content_type":"text/x-puppet","patch_set":4,"id":"ba15a1d1_56c16fe3","line":66,"in_reply_to":"ba15a1d1_bb547654","updated":"2015-09-14 13:00:31.000000000","message":"Oh, yeah, sorry, you\u0027re right, if we have SERVICE_DEFAULT for share_mode we don\u0027t need images_dir.","commit_id":"e391262f31b567373a2f9618b49832fba3678c7b"}],"manifests/volume/gpfs.pp":[{"author":{"_account_id":7745,"name":"Denis Egorenko","email":"degorenko@mirantis.com","username":"degorenko"},"change_message_id":"fbc0da6a97cb0045055708d3872a0bb3545b5cf8","unresolved":false,"context_lines":[{"line_number":61,"context_line":"  $gpfs_images_share_mode \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027,"},{"line_number":62,"context_line":"  $gpfs_max_clone_depth   \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027,"},{"line_number":63,"context_line":"  $gpfs_sparse_volumes    \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027,"},{"line_number":64,"context_line":"  $gpfs_storage_pool      \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027"},{"line_number":65,"context_line":") {"},{"line_number":66,"context_line":"  cinder::backend::gpfs { \u0027DEFAULT\u0027:"},{"line_number":67,"context_line":"    gpfs_mount_point_base  \u003d\u003e $gpfs_mount_point_base,"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"ba15a1d1_b1e25d9e","line":64,"updated":"2015-09-14 10:01:42.000000000","message":"Is it correct to set this parameter to \u003cSERVICE_DEFAULT\u003e? Because in docs above and prev patch sets it sets to \u0027system\u0027. The same for gpfs_sparse_volumes. The main propose of \u003cSERVICE_DEFAULT\u003e is that you don\u0027t need block of code with \u0027absent\u0027 setting. For those variables it also unneeded, because they have defaults. But may be i\u0027m wrong.","commit_id":"ff343e83396f1bcc339905c43087e7ec7cbdba5e"},{"author":{"_account_id":9410,"name":"Yanis Guenane","email":"yguenane@redhat.com","username":"yguenane"},"change_message_id":"2fb7632ed02ba024819339d57e8e97086eb1ce28","unresolved":false,"context_lines":[{"line_number":61,"context_line":"  $gpfs_images_share_mode \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027,"},{"line_number":62,"context_line":"  $gpfs_max_clone_depth   \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027,"},{"line_number":63,"context_line":"  $gpfs_sparse_volumes    \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027,"},{"line_number":64,"context_line":"  $gpfs_storage_pool      \u003d \u0027\u003cSERVICE DEFAULT\u003e\u0027"},{"line_number":65,"context_line":") {"},{"line_number":66,"context_line":"  cinder::backend::gpfs { \u0027DEFAULT\u0027:"},{"line_number":67,"context_line":"    gpfs_mount_point_base  \u003d\u003e $gpfs_mount_point_base,"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"ba15a1d1_b4524bdc","line":64,"in_reply_to":"ba15a1d1_b1e25d9e","updated":"2015-09-14 10:10:36.000000000","message":"Denis,\n\nYou are right about the documentation on this file, and the backend/gpfs.pp file, they are wrong as they don\u0027t match actual default.\n\nAs for the \u0027\u003cSERVICE DEFAULT\u003e\u0027 it aims to not have the ensure absent block that is true, resulting in the use of the upstream default. We do not want to specify the upstream default in the puppet code but rely directly on the value provided by upstream, as per the link Benedikt posted[1]\n\n[1] https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/ibm/gpfs.py","commit_id":"ff343e83396f1bcc339905c43087e7ec7cbdba5e"}],"spec/classes/cinder_volume_gpfs_spec.rb":[{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"c1bc517f30454dbb1f46bc631a2dee7711f009d8","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    before do"},{"line_number":59,"context_line":"      custom_params.merge(default_params)"},{"line_number":60,"context_line":"    end"},{"line_number":61,"context_line":"    it_configures \u0027gpfs volume driver\u0027"},{"line_number":62,"context_line":"  end"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  context \u0027with image share mode but without image path\u0027 do"}],"source_content_type":"text/x-ruby","patch_set":4,"id":"ba15a1d1_dd153740","line":61,"updated":"2015-09-16 12:45:59.000000000","message":"It seems that the custom_params is not used in the \u0027gpfs volume driver\u0027. Should the \u0027params.merge(custom_params)\u0027 be put here instead?","commit_id":"e391262f31b567373a2f9618b49832fba3678c7b"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"319ee0735b9bc1059837f35f8594035364f6b587","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        is_expected.to contain_cinder_config(\"DEFAULT/#{config}\").with_value( value )"},{"line_number":39,"context_line":"      end"},{"line_number":40,"context_line":"    end"},{"line_number":41,"context_line":"  end"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"  context \u0027with default parameters\u0027 do"},{"line_number":44,"context_line":"    it_configures \u0027gpfs volume driver\u0027"}],"source_content_type":"text/x-ruby","patch_set":5,"id":"ba15a1d1_9f5c691a","line":41,"updated":"2015-09-24 00:39:06.000000000","message":"This should just check whether cinder::backend::gpfs is in the catalog, checking the actual cinder_configs is already done in the tests for cinder::backend::gpfs.","commit_id":"83485bf05e0ca5a9180e781ea0668463634f34a4"}],"spec/defines/cinder_backend_gpfs_spec.rb":[{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"c1bc517f30454dbb1f46bc631a2dee7711f009d8","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"  context \u0027with custom parameters\u0027 do"},{"line_number":60,"context_line":"    before do"},{"line_number":61,"context_line":"      custom_params.merge(default_params)"},{"line_number":62,"context_line":"    end"},{"line_number":63,"context_line":"    it_configures \u0027gpfs volume driver\u0027"},{"line_number":64,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":4,"id":"ba15a1d1_5d9da7ba","line":61,"updated":"2015-09-16 12:45:59.000000000","message":"ditto","commit_id":"e391262f31b567373a2f9618b49832fba3678c7b"}]}