)]}'
{"manifests/wsgi/apache.pp":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"23fdefd3968b7bf3525a871a8220394fc4b006d5","unresolved":false,"context_lines":[{"line_number":116,"context_line":"  $ssl_crl                     \u003d undef,"},{"line_number":117,"context_line":"  $ssl_certs_dir               \u003d undef,"},{"line_number":118,"context_line":"  $wsgi_process_display_name   \u003d undef,"},{"line_number":119,"context_line":"  $wsgi_script_source          \u003d \u0027/usr/bin/cloudkitty-api\u0027,"},{"line_number":120,"context_line":"  $threads                     \u003d 1,"},{"line_number":121,"context_line":"  $priority                    \u003d \u002710\u0027,"},{"line_number":122,"context_line":"  $access_log_file             \u003d false,"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"9f560f44_01a7970b","line":119,"range":{"start_line":119,"start_character":33,"end_line":119,"end_character":59},"updated":"2020-09-29 23:44:55.000000000","message":"Can we keep the definition in cloudkitty::params and use the value defined here ?\n\nI know that we use the same value in all distro now, but it would help us to update when any distro change this path.","commit_id":"2b3acda88a680296a705a3536bb0e7de6687f778"},{"author":{"_account_id":7888,"name":"Benedikt Trefzer","email":"benedikt.trefzer@cirrax.com","username":"trefzer"},"change_message_id":"1f4996097b4590604089f06617d7513551db2756","unresolved":false,"context_lines":[{"line_number":116,"context_line":"  $ssl_crl                     \u003d undef,"},{"line_number":117,"context_line":"  $ssl_certs_dir               \u003d undef,"},{"line_number":118,"context_line":"  $wsgi_process_display_name   \u003d undef,"},{"line_number":119,"context_line":"  $wsgi_script_source          \u003d \u0027/usr/bin/cloudkitty-api\u0027,"},{"line_number":120,"context_line":"  $threads                     \u003d 1,"},{"line_number":121,"context_line":"  $priority                    \u003d \u002710\u0027,"},{"line_number":122,"context_line":"  $access_log_file             \u003d false,"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"9f560f44_41fb4fbd","line":119,"range":{"start_line":119,"start_character":33,"end_line":119,"end_character":59},"in_reply_to":"9f560f44_01a7970b","updated":"2020-09-30 00:09:20.000000000","message":"If you insist.... \nBut I think we should get rid of the params class anyway and use the data (hiera) directory to configure these things in proper, current puppet style.","commit_id":"2b3acda88a680296a705a3536bb0e7de6687f778"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2c2a6b1e74df469efad5ac2ee4035e45ff9b5b43","unresolved":false,"context_lines":[{"line_number":116,"context_line":"  $ssl_crl                     \u003d undef,"},{"line_number":117,"context_line":"  $ssl_certs_dir               \u003d undef,"},{"line_number":118,"context_line":"  $wsgi_process_display_name   \u003d undef,"},{"line_number":119,"context_line":"  $wsgi_script_source          \u003d \u0027/usr/bin/cloudkitty-api\u0027,"},{"line_number":120,"context_line":"  $threads                     \u003d 1,"},{"line_number":121,"context_line":"  $priority                    \u003d \u002710\u0027,"},{"line_number":122,"context_line":"  $access_log_file             \u003d false,"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"9f560f44_e1a6e3c0","line":119,"range":{"start_line":119,"start_character":33,"end_line":119,"end_character":59},"in_reply_to":"9f560f44_41fb4fbd","updated":"2020-09-30 00:27:01.000000000","message":"Sorry but I didn\u0027t get what you mean by \"proper, curent puppet style\"\n\nFrom implementation perspective we can use the parameter in cloudkitty::params class to define the default value, while we allow it to be overridden by using the parameter.\n\nclass cloudkitty::wsgi::apache (\n  ...\n  $wsgi_script_source \u003d $::cloudkitty::params::wsgi_script_source\n  ...\n) inherits cloudkitty::params {\n\n  include cloudkitty::deps\n  include apache\n}\n\nDo you have any concerns about the above direction ?","commit_id":"2b3acda88a680296a705a3536bb0e7de6687f778"},{"author":{"_account_id":7888,"name":"Benedikt Trefzer","email":"benedikt.trefzer@cirrax.com","username":"trefzer"},"change_message_id":"ddb6e035bc89ac95e2884a3da3964c293dd6a7bb","unresolved":false,"context_lines":[{"line_number":116,"context_line":"  $ssl_crl                     \u003d undef,"},{"line_number":117,"context_line":"  $ssl_certs_dir               \u003d undef,"},{"line_number":118,"context_line":"  $wsgi_process_display_name   \u003d undef,"},{"line_number":119,"context_line":"  $wsgi_script_source          \u003d \u0027/usr/bin/cloudkitty-api\u0027,"},{"line_number":120,"context_line":"  $threads                     \u003d 1,"},{"line_number":121,"context_line":"  $priority                    \u003d \u002710\u0027,"},{"line_number":122,"context_line":"  $access_log_file             \u003d false,"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"9f560f44_968e46cf","line":119,"range":{"start_line":119,"start_character":33,"end_line":119,"end_character":59},"in_reply_to":"9f560f44_e1a6e3c0","updated":"2020-09-30 07:52:45.000000000","message":"class inheritance is old style, used in puppet \u003c 4.\n(see https://puppet.com/docs/puppet/6.18/style_guide.html#class-inheritance)\n\nin puppet \u003e 4, you should use a hiera module hierarchy to set defaults. Since we require puppet \u003e 6, we should start implementing that. It would make the code faster and better readable.\n\nSo this is a style question, the old one like you propose or the \"proper, current\" or let\u0027s say \"modern\" according to the puppet style guide.\n\nBoth styles are working, but I have a strong recomandation, that we should go forward and implement towards a modern style.","commit_id":"2b3acda88a680296a705a3536bb0e7de6687f778"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"095a5b76e52a2ca5cb1821d864d938f88b18ccc8","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    wsgi_daemon_process         \u003d\u003e \u0027cloudkitty\u0027,"},{"line_number":155,"context_line":"    wsgi_process_display_name   \u003d\u003e $wsgi_process_display_name,"},{"line_number":156,"context_line":"    wsgi_process_group          \u003d\u003e \u0027cloudkitty\u0027,"},{"line_number":157,"context_line":"    wsgi_script_dir             \u003d\u003e $::cloudkitty::params::cloudkitty_wsgi_script_path,"},{"line_number":158,"context_line":"    wsgi_script_file            \u003d\u003e \u0027app\u0027,"},{"line_number":159,"context_line":"    wsgi_script_source          \u003d\u003e $wsgi_script_source,"},{"line_number":160,"context_line":"    custom_wsgi_process_options \u003d\u003e $custom_wsgi_process_options,"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"9f560f44_4166afbd","line":157,"range":{"start_line":157,"start_character":35,"end_line":157,"end_character":85},"updated":"2020-09-29 23:59:39.000000000","message":"Also, does it make sense to add a new parameter for this as well ?\n\nI see the similar fix has been merged into puppet-aodh and in that change this value was made configurable.","commit_id":"2b3acda88a680296a705a3536bb0e7de6687f778"},{"author":{"_account_id":7888,"name":"Benedikt Trefzer","email":"benedikt.trefzer@cirrax.com","username":"trefzer"},"change_message_id":"1f4996097b4590604089f06617d7513551db2756","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    wsgi_daemon_process         \u003d\u003e \u0027cloudkitty\u0027,"},{"line_number":155,"context_line":"    wsgi_process_display_name   \u003d\u003e $wsgi_process_display_name,"},{"line_number":156,"context_line":"    wsgi_process_group          \u003d\u003e \u0027cloudkitty\u0027,"},{"line_number":157,"context_line":"    wsgi_script_dir             \u003d\u003e $::cloudkitty::params::cloudkitty_wsgi_script_path,"},{"line_number":158,"context_line":"    wsgi_script_file            \u003d\u003e \u0027app\u0027,"},{"line_number":159,"context_line":"    wsgi_script_source          \u003d\u003e $wsgi_script_source,"},{"line_number":160,"context_line":"    custom_wsgi_process_options \u003d\u003e $custom_wsgi_process_options,"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"9f560f44_a1298b3b","line":157,"range":{"start_line":157,"start_character":35,"end_line":157,"end_character":85},"in_reply_to":"9f560f44_4166afbd","updated":"2020-09-30 00:09:20.000000000","message":"can do that.\nWould you agree, if I use a data/hiera hierarchy to set the defaults (and remove params.pp) ?","commit_id":"2b3acda88a680296a705a3536bb0e7de6687f778"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"628f5ab33cfebf7f8ea55cd1ba7d6c428dced559","unresolved":false,"context_lines":[{"line_number":120,"context_line":"  $ssl_crl                     \u003d undef,"},{"line_number":121,"context_line":"  $ssl_certs_dir               \u003d undef,"},{"line_number":122,"context_line":"  $wsgi_process_display_name   \u003d undef,"},{"line_number":123,"context_line":"  $wsgi_script_source          \u003d undef,"},{"line_number":124,"context_line":"  $wsgi_script_dir             \u003d undef,"},{"line_number":125,"context_line":"  $threads                     \u003d 1,"},{"line_number":126,"context_line":"  $priority                    \u003d \u002710\u0027,"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"9f560f44_80e36ad8","line":123,"range":{"start_line":123,"start_character":2,"end_line":123,"end_character":39},"updated":"2020-10-03 13:29:26.000000000","message":"One concern with current implementation is that it accepts \"undef\"  to use the default value.\nI don\u0027t know how many users will try using undef actually, but since we once accept it, we might need a deprecation phase as I did in https://review.opendev.org/#/c/755140/ .\n\nIf you will never implement this with class inheritance now then it would be better to implement using hiera now, otherwise we should somehow implement something to deal with this unexpected but valid usage...","commit_id":"e06c3b590faf2479e8f863940a24b923964dce5a"},{"author":{"_account_id":7888,"name":"Benedikt Trefzer","email":"benedikt.trefzer@cirrax.com","username":"trefzer"},"change_message_id":"99975efbed5b669d9f0788d2a5d96df37106af85","unresolved":false,"context_lines":[{"line_number":120,"context_line":"  $ssl_crl                     \u003d undef,"},{"line_number":121,"context_line":"  $ssl_certs_dir               \u003d undef,"},{"line_number":122,"context_line":"  $wsgi_process_display_name   \u003d undef,"},{"line_number":123,"context_line":"  $wsgi_script_source          \u003d undef,"},{"line_number":124,"context_line":"  $wsgi_script_dir             \u003d undef,"},{"line_number":125,"context_line":"  $threads                     \u003d 1,"},{"line_number":126,"context_line":"  $priority                    \u003d \u002710\u0027,"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"9f560f44_609e1607","line":123,"range":{"start_line":123,"start_character":2,"end_line":123,"end_character":39},"in_reply_to":"9f560f44_80e36ad8","updated":"2020-10-03 14:34:49.000000000","message":"Ack, correct (or bettern, modern as we said in this discussion) would be to add a type to the parameter (String) and hiera defaults per os-family. And remove the pick below.","commit_id":"e06c3b590faf2479e8f863940a24b923964dce5a"}]}