)]}'
{"ironic/common/images.py":[{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"0846ee59cd18a364c4009cc395126729d78c73e0","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        conf \u003d _generate_elilo_conf(kernel_params)"},{"line_number":249,"context_line":"        utils.write_to_file(elilo_efi_conf, conf)"},{"line_number":250,"context_line":"        uefi_pxe_bootfile_name \u003d os.path.join(CONF.pxe.tftp_root, \u0027/\u0027,"},{"line_number":251,"context_line":"                                             CONF.pxe.uefi_pxe_bootfile_name)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        efi_files_info \u003d {"},{"line_number":254,"context_line":"                          kernel: EFI_KERNEL,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_6ae6d0b1","line":251,"updated":"2014-09-04 00:32:24.000000000","message":"I don\u0027t think this will work. You don\u0027t need the \u0027/\u0027, os.path.join adds that for you.\n\n\u003e\u003e\u003e os.path.join(\u0027foo\u0027, \u0027/\u0027, \u0027bar\u0027)\n\n\u0027/bar\u0027","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"0846ee59cd18a364c4009cc395126729d78c73e0","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        efi_image_size \u003d os.path.getsize(kernel) + os.path.getsize(ramdisk)"},{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":265,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_6a333062","line":262,"updated":"2014-09-04 00:32:24.000000000","message":"So i get the /1000 for KiB, but why the +10? Padding?","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"548d26067a27ca9027356b5bb1d2500041cb60e0","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        efi_image_size \u003d os.path.getsize(kernel) + os.path.getsize(ramdisk)"},{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":265,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_c0e6b8af","line":262,"in_reply_to":"fa98f980_6a333062","updated":"2014-09-04 05:54:07.000000000","message":"yes, this is for padding.","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                help\u003d\u0027Template file for isolinux configuration file.\u0027),"},{"line_number":53,"context_line":"    cfg.StrOpt(\u0027elilo_efi_config_template\u0027,"},{"line_number":54,"context_line":"                default\u003dpaths.basedir_def(\u0027common/elilo_efi_config.template\u0027),"},{"line_number":55,"context_line":"                help\u003d\u0027Template file for elilo.efi configuration file.\u0027),"},{"line_number":56,"context_line":"]"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_6185fd05","line":55,"updated":"2014-09-04 15:50:14.000000000","message":"I don\u0027t see the sample config file in the list of files for this patch, adding a conf option requires you to update the sample config file.\n\nYou can do that by running:\n\n$ ./tools/config/generate_sample.sh","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"48a15a5e1b98bae8a1cb7e0291aabebc013fc21f","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                help\u003d\u0027Template file for isolinux configuration file.\u0027),"},{"line_number":53,"context_line":"    cfg.StrOpt(\u0027elilo_efi_config_template\u0027,"},{"line_number":54,"context_line":"                default\u003dpaths.basedir_def(\u0027common/elilo_efi_config.template\u0027),"},{"line_number":55,"context_line":"                help\u003d\u0027Template file for elilo.efi configuration file.\u0027),"},{"line_number":56,"context_line":"]"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_d9312e5b","line":55,"in_reply_to":"fa98f980_6185fd05","updated":"2014-09-08 05:57:37.000000000","message":"Done.","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":258,"context_line":"                         }"},{"line_number":259,"context_line":"        efi_image_size \u003d os.path.getsize(kernel) + os.path.getsize(ramdisk)"},{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_01d7d124","line":261,"updated":"2014-09-04 15:50:14.000000000","message":"conf is the string generated by _generate_elilo_conf() no?\n\nThis is going to fail I suppose.\n\nDid you mean \"elilo_efi_conf\" here? (which is the path for the file)","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"48a15a5e1b98bae8a1cb7e0291aabebc013fc21f","unresolved":false,"context_lines":[{"line_number":258,"context_line":"                         }"},{"line_number":259,"context_line":"        efi_image_size \u003d os.path.getsize(kernel) + os.path.getsize(ramdisk)"},{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_f9e6f2e6","line":261,"in_reply_to":"fa98f980_01d7d124","updated":"2014-09-08 05:57:37.000000000","message":"Done.","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        efi_image_size \u003d os.path.getsize(kernel) + os.path.getsize(ramdisk)"},{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":265,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_81d4a1ec","line":262,"updated":"2014-09-04 15:50:14.000000000","message":"why +10?\n\nI don\u0027t like this magic numbers, please put a comment why +10.\n\nAlso if you are converting it to KiB you should divide by 1024\n\n1 KiB \u003d\u003d 1024 bytes","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"48a15a5e1b98bae8a1cb7e0291aabebc013fc21f","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        efi_image_size \u003d os.path.getsize(kernel) + os.path.getsize(ramdisk)"},{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":265,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_39fb1a7a","line":262,"in_reply_to":"fa98f980_81d4a1ec","updated":"2014-09-08 05:57:37.000000000","message":"we might lose fraction of bytes while doing integer division. \nI have added a comment in the code.","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":265,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"},{"line_number":266,"context_line":"                           parameters\u003dNone, fs_size_kib\u003defi_image_size)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_61389d3d","line":263,"updated":"2014-09-04 15:50:14.000000000","message":"All this getsize() I think we can simplify this no?\n\ne.g\n\n efi_image_files \u003d [kernel, ramdisk, uefi_pxe_bootfile_name, elilo_efi_conf]\n efi_image_size \u003d sum([os.path.getsize(f) for f in efi_image_files]) / 1024","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"48a15a5e1b98bae8a1cb7e0291aabebc013fc21f","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        efi_image_size +\u003d os.path.getsize(uefi_pxe_bootfile_name)"},{"line_number":261,"context_line":"        efi_image_size +\u003d os.path.getsize(conf)"},{"line_number":262,"context_line":"        efi_image_size \u003d efi_image_size / 1000 + 10"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":265,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"},{"line_number":266,"context_line":"                           parameters\u003dNone, fs_size_kib\u003defi_image_size)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_b92e2af9","line":263,"in_reply_to":"fa98f980_61389d3d","updated":"2014-09-08 05:57:37.000000000","message":"Thanks for this simplification.\nIncorporated this in the code.","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4a8ada9b70140ddaa9f64635b451ac8d741172f1","unresolved":false,"context_lines":[{"line_number":262,"context_line":"        efi_image_size \u003d sum([os.path.getsize(f) for f in efi_image_files])"},{"line_number":263,"context_line":"        #Adding 4k to efi_image_size to compensate loss of faractions"},{"line_number":264,"context_line":"        #in integer division"},{"line_number":265,"context_line":"        efi_image_size \u003d efi_image_size / 1024 + 4"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":268,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_d549b833","line":265,"updated":"2014-09-08 09:27:40.000000000","message":"Just a nit: You can round it up in this case\n\n import math\n efi_image_size \u003d match.ceil(efi_image_size / float(1024))","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"da71e8f9be3adbb8357bc0b9081e29e0765bc7bc","unresolved":false,"context_lines":[{"line_number":262,"context_line":"        efi_image_size \u003d sum([os.path.getsize(f) for f in efi_image_files])"},{"line_number":263,"context_line":"        #Adding 4k to efi_image_size to compensate loss of faractions"},{"line_number":264,"context_line":"        #in integer division"},{"line_number":265,"context_line":"        efi_image_size \u003d efi_image_size / 1024 + 4"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":268,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_da59ce8f","line":265,"in_reply_to":"fa98f980_d549b833","updated":"2014-09-08 22:16:58.000000000","message":"I also need to add space to create directory paths, EFI/BOOT in this image, also around 1-2% of total size seems to be used while creating vfat partition. So I will take 2% of additional memory for this image.\n\nI will put comments in the code why I need this extra space to the image file.","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    return cfg"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"def _generate_elilo_conf(kernel_params):"},{"line_number":173,"context_line":"    \"\"\"Generates an elilo configuration file."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    Given a list of strings containing kernel parameters, this method"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_f814bbea","line":172,"updated":"2014-09-09 17:59:17.000000000","message":"The code for this is very similar to _generate_isolinux_cfg(). Maybe create a helper function for them?","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        \u0027K\u003dV\u0027 or \u0027K\u0027 or combination of them like \u0027K1\u003dV1 K2 K3\u003dV3\u0027) to be added"},{"line_number":179,"context_line":"        as the kernel cmdline."},{"line_number":180,"context_line":"    :returns: a string containing the contents of the isolinux configuration"},{"line_number":181,"context_line":"        file."},{"line_number":182,"context_line":"    \"\"\""},{"line_number":183,"context_line":"    if not kernel_params:"},{"line_number":184,"context_line":"        kernel_params \u003d []"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_78444b1b","line":181,"updated":"2014-09-09 17:59:17.000000000","message":"isolinux?","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    Copies the provided kernel, ramdisk to a directory, generates the isolinux"},{"line_number":203,"context_line":"    configuration file using the kernel parameters provided, and then generates"},{"line_number":204,"context_line":"    a bootable ISO image."},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    :param output_file: the path to the file where the iso image needs to be"},{"line_number":207,"context_line":"        created."}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_58b4e7f1","line":204,"updated":"2014-09-09 17:59:17.000000000","message":"There\u0027s nothing in this description about efi boot image. Is it worth mentioning?","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        isolinux_cfg \u003d os.path.join(tmpdir, ISOLINUX_CFG)"},{"line_number":242,"context_line":"        utils.write_to_file(isolinux_cfg, cfg)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        #Add efiboot.img to this iso image to support UEFI boot."},{"line_number":245,"context_line":"        elilo_efi_conf \u003d os.path.join(tmpdir, TEMP_ELILO_CONF)"},{"line_number":246,"context_line":"        conf \u003d _generate_elilo_conf(kernel_params)"},{"line_number":247,"context_line":"        utils.write_to_file(elilo_efi_conf, conf)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_1b209caf","line":244,"updated":"2014-09-09 17:59:17.000000000","message":"Please add a white space after \u0027#\u0027 (everywhere you\u0027ve added comments). That will make it more consistent with the rest of the code.","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b668e071e09b0c6c10555205cd2e3033c8dbabda","unresolved":false,"context_lines":[{"line_number":246,"context_line":"        conf \u003d _generate_elilo_conf(kernel_params)"},{"line_number":247,"context_line":"        utils.write_to_file(elilo_efi_conf, conf)"},{"line_number":248,"context_line":"        uefi_pxe_bootfile_name \u003d os.path.join(CONF.pxe.tftp_root,"},{"line_number":249,"context_line":"                                             CONF.pxe.uefi_pxe_bootfile_name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        efi_files_info \u003d {"},{"line_number":252,"context_line":"                          kernel: EFI_KERNEL,"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_4d251809","line":249,"updated":"2014-09-09 07:44:05.000000000","message":"nit: indent 2 spaces to the right to match bracket","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b668e071e09b0c6c10555205cd2e3033c8dbabda","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        #Todo(Faizan): Adding additional 2% of total file size for creation"},{"line_number":262,"context_line":"        #of vfat partition and additional directories apart from the files"},{"line_number":263,"context_line":"        #being copied. We can tune this padding later."},{"line_number":264,"context_line":"        efi_image_size \u003d (efi_image_size + (efi_image_size * 2 / 100)) / 1024"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":267,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_0d0cf093","line":264,"updated":"2014-09-09 07:44:05.000000000","message":"in Python the result of deletion will be rounded down, which maybe not what you want. I suggest switching to floats and then rounding up:\n\n efi_image_size \u003d (efi_image_size + (float(efi_image_size) * 2 / 100))\n # Switch to integer KiB\n efi_image_size \u003d math.ceil(efi_image_size / 1024)","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b668e071e09b0c6c10555205cd2e3033c8dbabda","unresolved":false,"context_lines":[{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        efiboot_img \u003d os.path.join(tmpdir, EFIBOOT_IMG)"},{"line_number":267,"context_line":"        create_vfat_image(efiboot_img, files_info\u003defi_files_info,"},{"line_number":268,"context_line":"                           parameters\u003dNone, fs_size_kib\u003defi_image_size)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        try:"},{"line_number":271,"context_line":"            utils.execute(\u0027mkisofs\u0027, \u0027-r\u0027, \u0027-V\u0027, \"BOOT IMAGE\","}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_cd7828fb","line":268,"updated":"2014-09-09 07:44:05.000000000","message":"nit: indent 1 space to the left to match bracket","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":273,"context_line":"                          \u0027-boot-load-size\u0027, \u00274\u0027, \u0027-boot-info-table\u0027,"},{"line_number":274,"context_line":"                          \u0027-b\u0027, ISOLINUX_BIN, \u0027-eltorito-alt-boot\u0027,"},{"line_number":275,"context_line":"                          \u0027-e\u0027, EFIBOOT_IMG, \u0027-no-emul-boot\u0027,"},{"line_number":276,"context_line":"                          \u0027-o\u0027, output_file, tmpdir)"},{"line_number":277,"context_line":"        except processutils.ProcessExecutionError as e:"},{"line_number":278,"context_line":"            LOG.exception(_LE(\"Creating ISO image failed.\"))"},{"line_number":279,"context_line":"            raise exception.ImageCreationFailed(image_type\u003d\u0027iso\u0027, error\u003de)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_98f72f8c","line":276,"updated":"2014-09-09 17:59:17.000000000","message":"2 dumb questions.\n\nDo we always want to put the efi boot image in?\n\nI saw something [1] that mentioned that not all \u0027mkisofs\u0027 supports the -e option. Do we need to check if it is supported or not?\n\n[1] http://wiki.osdev.org/Mkisofs#Options_and_input_files","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cdc06251a25c798a0c0623331e6ae7f079dcf8cd","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"def _generate_conf_file(kernel_params, template):"},{"line_number":147,"context_line":"    \"\"\"Generates a configuration file."},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    Given a given a list of strings containing kernel parameters, this method"},{"line_number":150,"context_line":"    returns the kernel cmdline string."}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_c360cd37","line":147,"updated":"2014-09-10 13:21:59.000000000","message":"nit: I would prefer it like \"Generates a configuration file for either isolinux or elilo\". It\u0027s too generic as it is now.","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ca5c85041754ddf3a98572a0ba1bc53a2f8fe027","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"def _generate_conf_file(kernel_params, template):"},{"line_number":147,"context_line":"    \"\"\"Generates a configuration file."},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    Given a given a list of strings containing kernel parameters, this method"},{"line_number":150,"context_line":"    returns the kernel cmdline string."}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_ba3767af","line":147,"in_reply_to":"fa98f980_c360cd37","updated":"2014-09-10 16:50:34.000000000","message":"Actually, I think this needs to be made clearer. It doesn\u0027t actually generate a FILE.\n\nI have two suggestions:\n1. move the code that actually generates the file to here. (More work, maybe cleaner). Eg, pass the directory and file-name as arguments, and add (something like):\n    cfg_file \u003d os.path.join(directory, file_name)\n    ...\n    utils.write_to_file(cfg_file, cfg)\n\n2. Rename this method and update the docstring. Eg, rename to _generate_config(), and \"Generates the contents for a configuration file for ...\".","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ca5c85041754ddf3a98572a0ba1bc53a2f8fe027","unresolved":false,"context_lines":[{"line_number":146,"context_line":"def _generate_conf_file(kernel_params, template):"},{"line_number":147,"context_line":"    \"\"\"Generates a configuration file."},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    Given a given a list of strings containing kernel parameters, this method"},{"line_number":150,"context_line":"    returns the kernel cmdline string."},{"line_number":151,"context_line":"    :param kernel_params: a list of strings(each element being a string like"},{"line_number":152,"context_line":"        \u0027K\u003dV\u0027 or \u0027K\u0027 or combination of them like \u0027K1\u003dV1 K2 K3\u003dV3\u0027) to be added"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_f7fe686a","line":149,"updated":"2014-09-10 16:50:34.000000000","message":"Nit: \u0027Given a given\u0027 -\u003e \u0027Given\u0027","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ca5c85041754ddf3a98572a0ba1bc53a2f8fe027","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    Given a given a list of strings containing kernel parameters, this method"},{"line_number":150,"context_line":"    returns the kernel cmdline string."},{"line_number":151,"context_line":"    :param kernel_params: a list of strings(each element being a string like"},{"line_number":152,"context_line":"        \u0027K\u003dV\u0027 or \u0027K\u0027 or combination of them like \u0027K1\u003dV1 K2 K3\u003dV3\u0027) to be added"},{"line_number":153,"context_line":"        as the kernel cmdline."},{"line_number":154,"context_line":"    :param template : the config file template which is to be generated"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_373f1027","line":151,"updated":"2014-09-10 16:50:34.000000000","message":"nit: We typically put a blank line between the description and the parameters. (I guess it wasn\u0027t mentioned before.)\n\nAlso, maybe add a space between \"strings(each\" -\u003e \"strings (each\"","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ca5c85041754ddf3a98572a0ba1bc53a2f8fe027","unresolved":false,"context_lines":[{"line_number":183,"context_line":"    \"\"\""},{"line_number":184,"context_line":"    template \u003d CONF.isolinux_config_template"},{"line_number":185,"context_line":"    cfg \u003d _generate_conf_file(kernel_params, template)"},{"line_number":186,"context_line":"    return cfg"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"def _generate_elilo_conf(kernel_params):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_77aa781a","line":186,"updated":"2014-09-10 16:50:34.000000000","message":"This is all \u0027optimizing\u0027 here, but two thoughts came to mind (you don\u0027t need to do either, I\u0027m just mentioning it in case you like either idea):\n1. you may not even need this method. Whatever calls it, could just do\n    cfg \u003d _generate_conf_file(kernel_params, CONF.isolinux_config_template)\n\n2. if you want this method, you don\u0027t really need \u0027cfg\u0027, you could just \n   return _generate_conf_file(kernel_params, CONF.isolinux_config_template)","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"33d0920bfc88650d353179509d93ab1647183764","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                          \u0027-boot-load-size\u0027, \u00274\u0027, \u0027-boot-info-table\u0027,"},{"line_number":282,"context_line":"                          \u0027-b\u0027, ISOLINUX_BIN, \u0027-eltorito-alt-boot\u0027,"},{"line_number":283,"context_line":"                          \u0027-e\u0027, EFIBOOT_IMG, \u0027-no-emul-boot\u0027,"},{"line_number":284,"context_line":"                          \u0027-o\u0027, output_file, tmpdir)"},{"line_number":285,"context_line":"        except processutils.ProcessExecutionError as e:"},{"line_number":286,"context_line":"            LOG.exception(_LE(\"Creating ISO image failed.\"))"},{"line_number":287,"context_line":"            raise exception.ImageCreationFailed(image_type\u003d\u0027iso\u0027, error\u003de)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_3521d69f","line":284,"updated":"2014-09-10 09:39:52.000000000","message":"There\u0027s a non-answered question from Ruby (Patch-set #10) here","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ca5c85041754ddf3a98572a0ba1bc53a2f8fe027","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                          \u0027-boot-load-size\u0027, \u00274\u0027, \u0027-boot-info-table\u0027,"},{"line_number":282,"context_line":"                          \u0027-b\u0027, ISOLINUX_BIN, \u0027-eltorito-alt-boot\u0027,"},{"line_number":283,"context_line":"                          \u0027-e\u0027, EFIBOOT_IMG, \u0027-no-emul-boot\u0027,"},{"line_number":284,"context_line":"                          \u0027-o\u0027, output_file, tmpdir)"},{"line_number":285,"context_line":"        except processutils.ProcessExecutionError as e:"},{"line_number":286,"context_line":"            LOG.exception(_LE(\"Creating ISO image failed.\"))"},{"line_number":287,"context_line":"            raise exception.ImageCreationFailed(image_type\u003d\u0027iso\u0027, error\u003de)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_3a72977c","line":284,"in_reply_to":"fa98f980_3521d69f","updated":"2014-09-10 16:50:34.000000000","message":"Yes, actually I had two questions. I\u0027ll repeat them again:\n\n2 dumb questions.\n\nDo we always want to put the efi boot image in?\n\nI saw something [1] that mentioned that not all \u0027mkisofs\u0027 supports the -e option. Do we need to check if it is supported or not?\n\n[1] http://wiki.osdev.org/Mkisofs#Options_and_input_files","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"698242451b30c6afd4f43a4d86b00bcbbab2401b","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                          \u0027-cache-inodes\u0027, \u0027-J\u0027, \u0027-l\u0027, \u0027-no-emul-boot\u0027,"},{"line_number":249,"context_line":"                          \u0027-boot-load-size\u0027, \u00274\u0027, \u0027-boot-info-table\u0027,"},{"line_number":250,"context_line":"                          \u0027-b\u0027, ISOLINUX_BIN, \u0027-eltorito-alt-boot\u0027,"},{"line_number":251,"context_line":"                          \u0027-e\u0027, EFIBOOT_IMG, \u0027-no-emul-boot\u0027,"},{"line_number":252,"context_line":"                          \u0027-o\u0027, output_file, tmpdir)"},{"line_number":253,"context_line":"        except processutils.ProcessExecutionError as e:"},{"line_number":254,"context_line":"            LOG.exception(_LE(\"Creating ISO image failed.\"))"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_a91aaec3","line":251,"updated":"2014-09-11 08:40:48.000000000","message":"I think the main idea to check whether mkisofs supports the \"-e\" (efi) option is to avoid calling it here in case if it\u0027s not supported AND the node is not in uefi boot mode.\n\nOr the idea here is to _always_ create an ISO with efi support even if the node boot_mode is not UEFI?","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"3550b63d01e57577fed135520b4e8604ec9494a5","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                          \u0027-cache-inodes\u0027, \u0027-J\u0027, \u0027-l\u0027, \u0027-no-emul-boot\u0027,"},{"line_number":249,"context_line":"                          \u0027-boot-load-size\u0027, \u00274\u0027, \u0027-boot-info-table\u0027,"},{"line_number":250,"context_line":"                          \u0027-b\u0027, ISOLINUX_BIN, \u0027-eltorito-alt-boot\u0027,"},{"line_number":251,"context_line":"                          \u0027-e\u0027, EFIBOOT_IMG, \u0027-no-emul-boot\u0027,"},{"line_number":252,"context_line":"                          \u0027-o\u0027, output_file, tmpdir)"},{"line_number":253,"context_line":"        except processutils.ProcessExecutionError as e:"},{"line_number":254,"context_line":"            LOG.exception(_LE(\"Creating ISO image failed.\"))"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_42dcb7e4","line":251,"in_reply_to":"fa98f980_a91aaec3","updated":"2014-09-11 10:34:38.000000000","message":"Hi Lucas, Yes, here we wanted to create an ISO with efi support irrespective of the boot mode. But it looks like creating efi supported iso on the fly is becoming complicated. Also I was facing some issue with iso created in this way. \nSo I would like to back out these changes, to create efi supported boot-iso b y the driver, rather I would like operator or adim to provide the efi-capable boot-iso as a glance image, similar to providing deploy-iso which should also be capable of efi boot.\n\nI will revert back these changes to images.py and respective test cases.","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"}],"ironic/drivers/modules/ilo/common.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"486ea88408fd0d192151b55a33ed855f5b51af37","unresolved":false,"context_lines":[{"line_number":299,"context_line":"                          \"parameter \u0027%s\u0027.\") % boot_mode)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"def get_node_capability(node, capability):"},{"line_number":303,"context_line":"    \"\"\"Returns \u0027capability\u0027 value from node\u0027s \u0027capabilities\u0027 property"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    :param node: Node object."}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_1a8414b0","line":302,"updated":"2014-08-26 15:57:32.000000000","message":"this is being added in another patch of your. I suggest we add that patch as a dependency for this one to avoid duplication?\n\n https://review.openstack.org/#/c/114357/7/ironic/common/pxe_utils.py","commit_id":"f078ba61cb13cb65aa98717dc5b7cfa73f654aab"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"8c070238488f641d93f093d7e8400cd4b786db16","unresolved":false,"context_lines":[{"line_number":299,"context_line":"                          \"parameter \u0027%s\u0027.\") % boot_mode)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"def get_node_capability(node, capability):"},{"line_number":303,"context_line":"    \"\"\"Returns \u0027capability\u0027 value from node\u0027s \u0027capabilities\u0027 property"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    :param node: Node object."}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_28cb2ea6","line":302,"in_reply_to":"1abeadc6_1a8414b0","updated":"2014-08-27 07:21:50.000000000","message":"I may not give multiple dependent patches for this patch. Once the above patch gets merge I will re-use the common function from pxe_utils and remove these duplicate definitions.","commit_id":"f078ba61cb13cb65aa98717dc5b7cfa73f654aab"},{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"0846ee59cd18a364c4009cc395126729d78c73e0","unresolved":false,"context_lines":[{"line_number":344,"context_line":"                         p_boot_mode.replace(\u0027LEGACY\u0027, \u0027bios\u0027).lower())"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"def set_node_capability(task, capability, value):"},{"line_number":348,"context_line":"    \"\"\"Set \u0027capability\u0027 to node\u0027s \u0027capabilities\u0027 property."},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    If value is \u0027None\u0027, then remove the capability from node property"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_8cd5d11d","line":347,"updated":"2014-09-04 00:32:24.000000000","message":"In the future I\u0027d like to see this and get_node_capability in the Ironic common library. Should be fine to land for now though.","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"548d26067a27ca9027356b5bb1d2500041cb60e0","unresolved":false,"context_lines":[{"line_number":344,"context_line":"                         p_boot_mode.replace(\u0027LEGACY\u0027, \u0027bios\u0027).lower())"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"def set_node_capability(task, capability, value):"},{"line_number":348,"context_line":"    \"\"\"Set \u0027capability\u0027 to node\u0027s \u0027capabilities\u0027 property."},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    If value is \u0027None\u0027, then remove the capability from node property"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_3b7fb9ac","line":347,"in_reply_to":"fa98f980_8cd5d11d","updated":"2014-09-04 05:54:07.000000000","message":"Sure.","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"0846ee59cd18a364c4009cc395126729d78c73e0","unresolved":false,"context_lines":[{"line_number":363,"context_line":"    if value:"},{"line_number":364,"context_line":"        new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":365,"context_line":"    else:"},{"line_number":366,"context_line":"        new_cap \u003d \u0027\u0027"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    old_cap_value \u003d None"},{"line_number":369,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_2c3ea528","line":366,"updated":"2014-09-04 00:32:24.000000000","message":"I\u0027m pretty sure this would be a malformed capability.","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"548d26067a27ca9027356b5bb1d2500041cb60e0","unresolved":false,"context_lines":[{"line_number":363,"context_line":"    if value:"},{"line_number":364,"context_line":"        new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":365,"context_line":"    else:"},{"line_number":366,"context_line":"        new_cap \u003d \u0027\u0027"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    old_cap_value \u003d None"},{"line_number":369,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_fb78d1c5","line":366,"in_reply_to":"fa98f980_2c3ea528","updated":"2014-09-04 05:54:07.000000000","message":"This is used to remove the existing capability. \nYes, it may retain \",\" in the capabilities string, but will not cause any issues. Will fix it later","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"0846ee59cd18a364c4009cc395126729d78c73e0","unresolved":false,"context_lines":[{"line_number":382,"context_line":"            capabilities \u003d capabilities.replace(old_cap_value, new_cap)"},{"line_number":383,"context_line":"        else:"},{"line_number":384,"context_line":"            if value:"},{"line_number":385,"context_line":"                capabilities \u003d os.path.join(new_cap, \u0027,\u0027, capabilities)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                return None"},{"line_number":388,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_4cb7693a","line":385,"updated":"2014-09-04 00:32:24.000000000","message":"I don\u0027t think this is what you want.\n\n\u003e\u003e\u003e os.path.join(\u0027foo\u0027, \u0027,\u0027, \u0027baz\u0027)\n\n\u0027foo/,/baz\u0027\n\nMaybe you want something like \"capabilities \u003d new_cap + \u0027,\u0027 + capabilities\"","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"548d26067a27ca9027356b5bb1d2500041cb60e0","unresolved":false,"context_lines":[{"line_number":382,"context_line":"            capabilities \u003d capabilities.replace(old_cap_value, new_cap)"},{"line_number":383,"context_line":"        else:"},{"line_number":384,"context_line":"            if value:"},{"line_number":385,"context_line":"                capabilities \u003d os.path.join(new_cap, \u0027,\u0027, capabilities)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                return None"},{"line_number":388,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_3ba8991f","line":385,"in_reply_to":"fa98f980_4cb7693a","updated":"2014-09-04 05:54:07.000000000","message":"Done.","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":286,"context_line":"    \"\"\""},{"line_number":287,"context_line":"    if boot_mode is None:"},{"line_number":288,"context_line":"        LOG.info(_LI(\"No boot mode specified.\"))"},{"line_number":289,"context_line":"        return"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    ilo_object \u003d get_ilo_object(node)"},{"line_number":292,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_e1df2dd4","line":289,"updated":"2014-09-04 15:50:14.000000000","message":"I don\u0027t see how it\u0027s going to be called with None\n\nAt deploy.py it\u0027s like:\n\n if boot_mode is not None:\n     ilo_common.set_boot_mode(task.node, boot_mode)","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"48a15a5e1b98bae8a1cb7e0291aabebc013fc21f","unresolved":false,"context_lines":[{"line_number":286,"context_line":"    \"\"\""},{"line_number":287,"context_line":"    if boot_mode is None:"},{"line_number":288,"context_line":"        LOG.info(_LI(\"No boot mode specified.\"))"},{"line_number":289,"context_line":"        return"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    ilo_object \u003d get_ilo_object(node)"},{"line_number":292,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_343ddfc4","line":289,"in_reply_to":"fa98f980_e1df2dd4","updated":"2014-09-08 05:57:37.000000000","message":"I have added this for error handling.\n\nI will remove this code.","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":321,"context_line":"    \"\"\""},{"line_number":322,"context_line":"    boot_mode \u003d get_node_capability(node, \u0027boot_mode\u0027)"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    if boot_mode not in [None, \u0027bios\u0027, \u0027uefi\u0027]:"},{"line_number":325,"context_line":"        raise exception.InvalidParameterValue(_LE(\"Invalid boot_mode \""},{"line_number":326,"context_line":"                          \"parameter \u0027%s\u0027.\") % boot_mode)"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_61709ddc","line":324,"updated":"2014-09-04 15:50:14.000000000","message":"nit: \n\n if boot_mode and boot_mode not in (\u0027bios\u0027, \u0027uefi\u0027):\n     ...","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"48a15a5e1b98bae8a1cb7e0291aabebc013fc21f","unresolved":false,"context_lines":[{"line_number":321,"context_line":"    \"\"\""},{"line_number":322,"context_line":"    boot_mode \u003d get_node_capability(node, \u0027boot_mode\u0027)"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    if boot_mode not in [None, \u0027bios\u0027, \u0027uefi\u0027]:"},{"line_number":325,"context_line":"        raise exception.InvalidParameterValue(_LE(\"Invalid boot_mode \""},{"line_number":326,"context_line":"                          \"parameter \u0027%s\u0027.\") % boot_mode)"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_34b23f4c","line":324,"in_reply_to":"fa98f980_61709ddc","updated":"2014-09-08 05:57:37.000000000","message":"Done.","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":343,"context_line":"                         p_boot_mode.replace(\u0027LEGACY\u0027, \u0027bios\u0027).lower())"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"def set_node_capability(task, capability, value):"},{"line_number":347,"context_line":"    \"\"\"Set \u0027capability\u0027 to node\u0027s \u0027capabilities\u0027 property."},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    If value is \u0027None\u0027, then remove the capability from node property"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_c48f9773","line":346,"updated":"2014-09-04 15:50:14.000000000","message":"this function seems very generic and could be useful by other drivers than iLO can we put it in a common place?\n\nironic/drivers/utils.py seems like a good place for this","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"48a15a5e1b98bae8a1cb7e0291aabebc013fc21f","unresolved":false,"context_lines":[{"line_number":343,"context_line":"                         p_boot_mode.replace(\u0027LEGACY\u0027, \u0027bios\u0027).lower())"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"def set_node_capability(task, capability, value):"},{"line_number":347,"context_line":"    \"\"\"Set \u0027capability\u0027 to node\u0027s \u0027capabilities\u0027 property."},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    If value is \u0027None\u0027, then remove the capability from node property"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_14f61bf9","line":346,"in_reply_to":"fa98f980_c48f9773","updated":"2014-09-08 05:57:37.000000000","message":"Sure. will do that.","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":346,"context_line":"def set_node_capability(task, capability, value):"},{"line_number":347,"context_line":"    \"\"\"Set \u0027capability\u0027 to node\u0027s \u0027capabilities\u0027 property."},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    If value is \u0027None\u0027, then remove the capability from node property"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    :param task: Task object."},{"line_number":352,"context_line":"    :param capability: Capability key."}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_a4db737a","line":349,"updated":"2014-09-04 15:50:14.000000000","message":"Ins\u0027t it better to split it in 2 functions? The code looks pretty hard to read if you have both cases in one functions, what you think?\n\nE.g\n\n add_node_capability()\n rm_node_capabilitiy()","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":357,"context_line":"    capabilities \u003d node.properties.get(\u0027capabilities\u0027)"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    if capability is None:"},{"line_number":360,"context_line":"        return None"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    if value:"},{"line_number":363,"context_line":"        new_cap \u003d capability + \u0027:\u0027 + value"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_042a7f7e","line":360,"updated":"2014-09-04 15:50:14.000000000","message":"s/return None/return/g\n\nBut why? why we don\u0027t create the capabilities dict if it\u0027s not created yet?","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":383,"context_line":"            if value:"},{"line_number":384,"context_line":"                capabilities \u003d new_cap + \u0027,\u0027 + capabilities"},{"line_number":385,"context_line":"            else:"},{"line_number":386,"context_line":"                return None"},{"line_number":387,"context_line":"    else:"},{"line_number":388,"context_line":"        capabilities \u003d new_cap"},{"line_number":389,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_24a223fd","line":386,"updated":"2014-09-04 15:50:14.000000000","message":"nit: s/return None/return/g","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"223f4253513f9ada73d357e7c5fbd888525b0d00","unresolved":false,"context_lines":[{"line_number":391,"context_line":"    node.save(task.context)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"def get_node_capability(node, capability):"},{"line_number":395,"context_line":"    \"\"\"Returns \u0027capability\u0027 value from node\u0027s \u0027capabilities\u0027 property"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    :param node: Node object."}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_e4b81b13","line":394,"updated":"2014-09-04 15:50:14.000000000","message":"this also looks pretty generic, mind adding it to a common place where other drivers can use it as well?\n\nironic/drivers/utils.py seems like a good place for this","commit_id":"63d1f0e32537f88415b9e189385921f79f2c88bf"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"_LE \u003d i18n._LE"},{"line_number":61,"context_line":"_LI \u003d i18n._LI"},{"line_number":62,"context_line":"_LW \u003d i18n._LW"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"REQUIRED_PROPERTIES \u003d {"},{"line_number":65,"context_line":"    \u0027ilo_address\u0027: _(\"IP address or hostname of the iLO. Required.\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_9bae49e6","line":62,"updated":"2014-09-09 17:59:17.000000000","message":"This should be moved up, as \u0027from ironic.common.i18n import _LW\u0027\n\nYou\u0027ll see what I mean when you rebase.","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b668e071e09b0c6c10555205cd2e3033c8dbabda","unresolved":false,"context_lines":[{"line_number":290,"context_line":"    try:"},{"line_number":291,"context_line":"        p_boot_mode \u003d ilo_object.get_pending_boot_mode()"},{"line_number":292,"context_line":"    except ilo_client.IloCommandNotSupportedError:"},{"line_number":293,"context_line":"        p_boot_mode \u003d \u0027LEGACY\u0027"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    if p_boot_mode.lower().replace(\u0027legacy\u0027, \u0027bios\u0027) \u003d\u003d boot_mode:"},{"line_number":296,"context_line":"        LOG.info(_LI(\"Node %(uuid)s pending boot mode is %(boot_mode)s.\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_4d019838","line":293,"updated":"2014-09-09 07:44:05.000000000","message":"should be global constant like\n\n DEFAULT_BOOT_MODE \u003d \u0027LEGACY\u0027","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b668e071e09b0c6c10555205cd2e3033c8dbabda","unresolved":false,"context_lines":[{"line_number":292,"context_line":"    except ilo_client.IloCommandNotSupportedError:"},{"line_number":293,"context_line":"        p_boot_mode \u003d \u0027LEGACY\u0027"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    if p_boot_mode.lower().replace(\u0027legacy\u0027, \u0027bios\u0027) \u003d\u003d boot_mode:"},{"line_number":296,"context_line":"        LOG.info(_LI(\"Node %(uuid)s pending boot mode is %(boot_mode)s.\"),"},{"line_number":297,"context_line":"                 {\u0027uuid\u0027: node.uuid, \u0027boot_mode\u0027: boot_mode})"},{"line_number":298,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_edcb2c1c","line":295,"updated":"2014-09-09 07:44:05.000000000","message":"I would prefer some global dictionary(-ies) matching ilo mode with generic ones like\n\n BOOT_MODE_GENERIC_TO_ILO \u003d {\u0027bios\u0027: \u0027legacy\u0027, ...}\n BOOT_MODE_ILO_TO_GENERIC \u003d dict((v, k) for (k, v) in BOOT_MODE_GENERIC_TO_ILO.items())\n\nthis will also allow checking for correct boot mode before invoking ilo operation","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":325,"context_line":"    driver_utils.rm_node_capability(task, \u0027boot_mode\u0027)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    driver_utils.add_node_capability(task, \u0027boot_mode\u0027,"},{"line_number":328,"context_line":"                         p_boot_mode.replace(\u0027LEGACY\u0027, \u0027bios\u0027).lower())"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"def setup_vmedia_for_boot(task, boot_iso, parameters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_bb394d41","line":328,"updated":"2014-09-09 17:59:17.000000000","message":"so maybe this is out of scope, but if the current value for \u0027boot_mode\u0027 is the same as what you\u0027re adding, there\u0027s no need to remove and add.\n\nWhy not have a driver_utils.replace_node_capability(task, capability_key, capability_value). Or change add_node_capability() to check first before adding.","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"33d0920bfc88650d353179509d93ab1647183764","unresolved":false,"context_lines":[{"line_number":72,"context_line":"COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)"},{"line_number":73,"context_line":"DEFAULT_BOOT_MODE \u003d \u0027LEGACY\u0027"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"BOOT_MODE_GENERIC_TO_ILO \u003d {\u0027bios\u0027: \u0027legacy\u0027, \u0027BIOS\u0027: \u0027LEGACY\u0027}"},{"line_number":76,"context_line":"BOOT_MODE_ILO_TO_GENERIC \u003d dict((v, k)"},{"line_number":77,"context_line":"                           for (k, v) in BOOT_MODE_GENERIC_TO_ILO.items())"},{"line_number":78,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_da8ef9cf","line":75,"updated":"2014-09-10 09:39:52.000000000","message":":/\n\nWhy the dict with same key-value pairs but one lower case and the other one upper case?\n\nWould be easy to always call lower() (or upper()) before searching in the dict?\n\nWhat I see now is a mix of both cases, for e.g just changing the order where call lower():\n\n * On set_boot_mode():\n  \n p_boot_mode.lower().replace(\u0027legacy\u0027, BOOT_MODE_ILO_TO_GENERIC[\u0027legacy\u0027])\n\n\n * On update_boot_mode_capabilitiy():\n\n p_boot_mode.replace(\u0027LEGACY\u0027,BOOT_MODE_ILO_TO_GENERIC[\u0027LEGACY\u0027]).lower()","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cdc06251a25c798a0c0623331e6ae7f079dcf8cd","unresolved":false,"context_lines":[{"line_number":72,"context_line":"COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)"},{"line_number":73,"context_line":"DEFAULT_BOOT_MODE \u003d \u0027LEGACY\u0027"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"BOOT_MODE_GENERIC_TO_ILO \u003d {\u0027bios\u0027: \u0027legacy\u0027, \u0027BIOS\u0027: \u0027LEGACY\u0027}"},{"line_number":76,"context_line":"BOOT_MODE_ILO_TO_GENERIC \u003d dict((v, k)"},{"line_number":77,"context_line":"                           for (k, v) in BOOT_MODE_GENERIC_TO_ILO.items())"},{"line_number":78,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_c3bc4d6f","line":75,"in_reply_to":"fa98f980_da8ef9cf","updated":"2014-09-10 13:21:59.000000000","message":"right, it should be only lower-case and include all expected boot modes. No replace() call is needed below to use these dicts.","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cdc06251a25c798a0c0623331e6ae7f079dcf8cd","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        p_boot_mode \u003d DEFAULT_BOOT_MODE"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    if p_boot_mode.lower().replace(\u0027legacy\u0027,"},{"line_number":298,"context_line":"                 BOOT_MODE_ILO_TO_GENERIC[\u0027legacy\u0027]) \u003d\u003d boot_mode:"},{"line_number":299,"context_line":"        LOG.info(_LI(\"Node %(uuid)s pending boot mode is %(boot_mode)s.\"),"},{"line_number":300,"context_line":"                 {\u0027uuid\u0027: node.uuid, \u0027boot_mode\u0027: boot_mode})"},{"line_number":301,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_c3ee0d8b","line":298,"updated":"2014-09-10 13:21:59.000000000","message":"should be:\n\n if BOOT_MODE_ILO_TO_GENERIC[p_boot_mode.lower()] \u003d\u003d boot_mode:","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cdc06251a25c798a0c0623331e6ae7f079dcf8cd","unresolved":false,"context_lines":[{"line_number":303,"context_line":"    try:"},{"line_number":304,"context_line":"        ilo_object.set_pending_boot_mode("},{"line_number":305,"context_line":"                        boot_mode.replace(\u0027bios\u0027,"},{"line_number":306,"context_line":"                        BOOT_MODE_GENERIC_TO_ILO[\u0027bios\u0027]).upper())"},{"line_number":307,"context_line":"    except ilo_client.IloError as ilo_exception:"},{"line_number":308,"context_line":"        operation \u003d _(\"Setting %s as boot mode\") % boot_mode"},{"line_number":309,"context_line":"        raise exception.IloOperationError(operation\u003doperation,"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_23ec398f","line":306,"updated":"2014-09-10 13:21:59.000000000","message":"should be \n\n ilo_object.set_pending_boot_mode(BOOT_MODE_GENERIC_TO_ILO[boot_mode].upper())","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"33d0920bfc88650d353179509d93ab1647183764","unresolved":false,"context_lines":[{"line_number":324,"context_line":"    try:"},{"line_number":325,"context_line":"        p_boot_mode \u003d ilo_object.get_pending_boot_mode()"},{"line_number":326,"context_line":"    except ilo_client.IloCommandNotSupportedError:"},{"line_number":327,"context_line":"        p_boot_mode \u003d \u0027LEGACY\u0027"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"    driver_utils.rm_node_capability(task, \u0027boot_mode\u0027)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_3ae9c580","line":327,"updated":"2014-09-10 09:39:52.000000000","message":"DEFAULT_BOOT_MODE","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        p_boot_mode \u003d ilo_object.get_pending_boot_mode()"},{"line_number":324,"context_line":"        if p_boot_mode \u003d\u003d \u0027UNKNOWN\u0027:"},{"line_number":325,"context_line":"            ilo_object.set_pending_boot_mode(\u0027UEFI\u0027)"},{"line_number":326,"context_line":"            p_boot_mode \u003d \u0027UEFI\u0027"},{"line_number":327,"context_line":"    except ilo_client.IloCommandNotSupportedError:"},{"line_number":328,"context_line":"        p_boot_mode \u003d DEFAULT_BOOT_MODE"},{"line_number":329,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_99cff155","line":326,"updated":"2014-09-11 16:45:11.000000000","message":"Just wondering why you decide to set to UEFI instead of DEFAULT_BOOT_MODE in this case?","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        p_boot_mode \u003d ilo_object.get_pending_boot_mode()"},{"line_number":324,"context_line":"        if p_boot_mode \u003d\u003d \u0027UNKNOWN\u0027:"},{"line_number":325,"context_line":"            ilo_object.set_pending_boot_mode(\u0027UEFI\u0027)"},{"line_number":326,"context_line":"            p_boot_mode \u003d \u0027UEFI\u0027"},{"line_number":327,"context_line":"    except ilo_client.IloCommandNotSupportedError:"},{"line_number":328,"context_line":"        p_boot_mode \u003d DEFAULT_BOOT_MODE"},{"line_number":329,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_018a9b1a","line":326,"in_reply_to":"fa98f980_99cff155","updated":"2014-09-11 21:32:55.000000000","message":"ilo will return UNKNOWN for very corner case and this will happen only of the nodes which supports dual mode. On other machines which does not support UEFI, it will return CommandNotSupprotedError. It is highly likely that we will receive \"UNKNOWN\" on new machines which has default boot mode as UEFI. So we want to try setting boot mode with UEFI, if this call fails, then we anyway end-up with default boot mode, which is BIOS","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"38a896c0cb3e344dc210cd2f1f3cee6b1945cbc4","unresolved":false,"context_lines":[{"line_number":330,"context_line":"    driver_utils.rm_node_capability(task, \u0027boot_mode\u0027)"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    driver_utils.add_node_capability(task, \u0027boot_mode\u0027,"},{"line_number":333,"context_line":"                         BOOT_MODE_ILO_TO_GENERIC[p_boot_mode.lower()])"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"def setup_vmedia_for_boot(task, boot_iso, parameters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_fc646023","line":333,"updated":"2014-09-11 20:40:45.000000000","message":"Just a nit, but I think this line should only be indented four spaces. Only do this if you push another revision.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"ee72304068ff76e506d4c938ff83dae09128516d","unresolved":false,"context_lines":[{"line_number":330,"context_line":"    driver_utils.rm_node_capability(task, \u0027boot_mode\u0027)"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    driver_utils.add_node_capability(task, \u0027boot_mode\u0027,"},{"line_number":333,"context_line":"                         BOOT_MODE_ILO_TO_GENERIC[p_boot_mode.lower()])"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"def setup_vmedia_for_boot(task, boot_iso, parameters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_01a7f850","line":333,"in_reply_to":"fa98f980_fc646023","updated":"2014-09-11 21:48:24.000000000","message":"Sure will do it now","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d3989a8e21428673bcb14dcc19c4c27f6d49adb7","unresolved":false,"context_lines":[{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    try:"},{"line_number":323,"context_line":"        p_boot_mode \u003d ilo_object.get_pending_boot_mode()"},{"line_number":324,"context_line":"        if p_boot_mode \u003d\u003d \u0027UNKNOWN\u0027:"},{"line_number":325,"context_line":"            ilo_object.set_pending_boot_mode(\u0027UEFI\u0027)"},{"line_number":326,"context_line":"            p_boot_mode \u003d \u0027UEFI\u0027"},{"line_number":327,"context_line":"    except ilo_client.IloCommandNotSupportedError:"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_84a216fa","line":324,"updated":"2014-09-11 22:05:34.000000000","message":"If you do another revision, it would be nice to have a (I copy/pasted your reply):\n\n# NOTE(syed-or-whatever) ilo will return UNKNOWN for corner cases, and this happens only for the nodes which support dual mode. On other machines which do not support UEFI, it returns CommandNotSupportedError. It is highly likely that we will receive \"UNKNOWN\" on new machines which have default boot mode as UEFI. So we want to try setting boot mode to UEFI. If this fails, then we will use the default boot mode, which is BIOS.","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"},{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"6c47691a3d5284135ad53b330e0e442b117913dc","unresolved":false,"context_lines":[{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    try:"},{"line_number":323,"context_line":"        p_boot_mode \u003d ilo_object.get_pending_boot_mode()"},{"line_number":324,"context_line":"        if p_boot_mode \u003d\u003d \u0027UNKNOWN\u0027:"},{"line_number":325,"context_line":"            ilo_object.set_pending_boot_mode(\u0027UEFI\u0027)"},{"line_number":326,"context_line":"            p_boot_mode \u003d \u0027UEFI\u0027"},{"line_number":327,"context_line":"    except ilo_client.IloCommandNotSupportedError:"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_e8c0102d","line":324,"in_reply_to":"fa98f980_84a216fa","updated":"2014-09-12 04:42:04.000000000","message":"Ok , we will add this comment","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"}],"ironic/drivers/modules/ilo/deploy.py":[{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"38a896c0cb3e344dc210cd2f1f3cee6b1945cbc4","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        LOG.debug(\"Found boot_iso %s in Glance\", boot_iso_uuid)"},{"line_number":94,"context_line":"        return \u0027glance:%s\u0027 % boot_iso_uuid"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    #For uefi boot_mode, operator should provide efi capable boot-iso in glance"},{"line_number":97,"context_line":"    if driver_utils.get_node_capability(task.node, \u0027boot_mode\u0027) \u003d\u003d \u0027uefi\u0027:"},{"line_number":98,"context_line":"        LOG.error(_LE(\"Unable to find boot_iso in Glance, required to deploy \""},{"line_number":99,"context_line":"                      \"node %(node)s in UEFI boot mode.\"),"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_1c6a04ee","line":96,"updated":"2014-09-11 20:40:45.000000000","message":"I\u0027m curious why this is? Why can\u0027t Ironic build an EFI capable boot ISO? (this might be a dumb question)\n\nAlso, comments like this should be in the form: `# NOTE(nickname) For uefi...`","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"ee72304068ff76e506d4c938ff83dae09128516d","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        LOG.debug(\"Found boot_iso %s in Glance\", boot_iso_uuid)"},{"line_number":94,"context_line":"        return \u0027glance:%s\u0027 % boot_iso_uuid"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    #For uefi boot_mode, operator should provide efi capable boot-iso in glance"},{"line_number":97,"context_line":"    if driver_utils.get_node_capability(task.node, \u0027boot_mode\u0027) \u003d\u003d \u0027uefi\u0027:"},{"line_number":98,"context_line":"        LOG.error(_LE(\"Unable to find boot_iso in Glance, required to deploy \""},{"line_number":99,"context_line":"                      \"node %(node)s in UEFI boot mode.\"),"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_a1eaac71","line":96,"in_reply_to":"fa98f980_1c6a04ee","updated":"2014-09-11 21:48:24.000000000","message":"I tried to do it earlier, but seems to be complicated the code. \nAlso I faced some issues with elilo boot loader, It was not giving consistent result with elilo bootloader. \nWith this approach, operator/admin can choose any of the efi boot loader not necessary stick to elilo. \n\nwe thought it is better to leave it to admin to create efi capable iso while using UEFI boot mode deploy.\n\nAnyway I will change the comments in the code.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":297,"context_line":"        \"\"\"Prepare the deployment environment for this task\u0027s node."},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        :param task: a TaskManager instance containing the node to act on."},{"line_number":300,"context_line":"        \"\"\""},{"line_number":301,"context_line":"        boot_mode \u003d driver_utils.get_node_capability(task.node, \u0027boot_mode\u0027)"},{"line_number":302,"context_line":"        if boot_mode is not None:"},{"line_number":303,"context_line":"            ilo_common.set_boot_mode(task.node, boot_mode)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_f990fd44","line":300,"updated":"2014-09-11 16:45:11.000000000","message":"looks like ilo_common.set_boot_mode() can raise IloOperationError. Please add :raises: IloOperationError etc... here","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":297,"context_line":"        \"\"\"Prepare the deployment environment for this task\u0027s node."},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        :param task: a TaskManager instance containing the node to act on."},{"line_number":300,"context_line":"        \"\"\""},{"line_number":301,"context_line":"        boot_mode \u003d driver_utils.get_node_capability(task.node, \u0027boot_mode\u0027)"},{"line_number":302,"context_line":"        if boot_mode is not None:"},{"line_number":303,"context_line":"            ilo_common.set_boot_mode(task.node, boot_mode)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_01b8dbc6","line":300,"in_reply_to":"fa98f980_f990fd44","updated":"2014-09-11 21:32:55.000000000","message":"Done","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d3989a8e21428673bcb14dcc19c4c27f6d49adb7","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        return \u0027glance:%s\u0027 % boot_iso_uuid"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    #NOTE(faizan) For uefi boot_mode, operator should provide efi capable"},{"line_number":97,"context_line":"    #boot-iso in glance"},{"line_number":98,"context_line":"    if driver_utils.get_node_capability(task.node, \u0027boot_mode\u0027) \u003d\u003d \u0027uefi\u0027:"},{"line_number":99,"context_line":"        LOG.error(_LE(\"Unable to find boot_iso in Glance, required to deploy \""},{"line_number":100,"context_line":"                      \"node %(node)s in UEFI boot mode.\"),"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_e493e2a0","line":97,"updated":"2014-09-11 22:05:34.000000000","message":"We tend to add a space after the #. Please do the same for consistency. Thx.","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"},{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"6c47691a3d5284135ad53b330e0e442b117913dc","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        return \u0027glance:%s\u0027 % boot_iso_uuid"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    #NOTE(faizan) For uefi boot_mode, operator should provide efi capable"},{"line_number":97,"context_line":"    #boot-iso in glance"},{"line_number":98,"context_line":"    if driver_utils.get_node_capability(task.node, \u0027boot_mode\u0027) \u003d\u003d \u0027uefi\u0027:"},{"line_number":99,"context_line":"        LOG.error(_LE(\"Unable to find boot_iso in Glance, required to deploy \""},{"line_number":100,"context_line":"                      \"node %(node)s in UEFI boot mode.\"),"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_a8ca884c","line":97,"in_reply_to":"fa98f980_e493e2a0","updated":"2014-09-12 04:42:04.000000000","message":"Ok","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"}],"ironic/drivers/modules/pxe.py":[{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":277,"context_line":"        \"\"\""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"        #Check if mkisofs supports -e option or not."},{"line_number":280,"context_line":"        self._check_is_option_supported()"},{"line_number":281,"context_line":"        # Check the boot_mode capability parameter value."},{"line_number":282,"context_line":"        driver_utils.validate_boot_mode_capability(task.node)"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_5dc57c8c","line":280,"updated":"2014-09-11 12:51:06.000000000","message":"This call is not required as PXE deploy will be be calling image creation methods. \nReverting back these changes.","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"698242451b30c6afd4f43a4d86b00bcbbab2401b","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        \"\"\"This method tests if mkisofs supports -e option or not."},{"line_number":309,"context_line":"        This is to make sure the image creation doesn\u0027t fail when"},{"line_number":310,"context_line":"        mkisofs is called with -e option to create UEFI boot capable"},{"line_number":311,"context_line":"        image."},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        \"\"\""},{"line_number":314,"context_line":"        output, err \u003d processutils.execute(\u0027mkisofs\u0027, \u0027--help\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_09ae5a81","line":311,"updated":"2014-09-11 08:40:48.000000000","message":"super nit: pep 257","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"3550b63d01e57577fed135520b4e8604ec9494a5","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        \"\"\"This method tests if mkisofs supports -e option or not."},{"line_number":309,"context_line":"        This is to make sure the image creation doesn\u0027t fail when"},{"line_number":310,"context_line":"        mkisofs is called with -e option to create UEFI boot capable"},{"line_number":311,"context_line":"        image."},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        \"\"\""},{"line_number":314,"context_line":"        output, err \u003d processutils.execute(\u0027mkisofs\u0027, \u0027--help\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_a2df23d5","line":311,"in_reply_to":"fa98f980_09ae5a81","updated":"2014-09-11 10:34:38.000000000","message":"This function will not be needed once I remove the images.py changes.","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"698242451b30c6afd4f43a4d86b00bcbbab2401b","unresolved":false,"context_lines":[{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        \"\"\""},{"line_number":314,"context_line":"        output, err \u003d processutils.execute(\u0027mkisofs\u0027, \u0027--help\u0027)"},{"line_number":315,"context_line":"        is_supported \u003d False"},{"line_number":316,"context_line":"        if \u0027-e\u0027 or \u0027EFI\u0027 in err:"},{"line_number":317,"context_line":"                    is_supported \u003d True"},{"line_number":318,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_e9bcb6bd","line":315,"updated":"2014-09-11 08:40:48.000000000","message":"We can make it set if it\u0027s supported globally so we don\u0027t have to keep checking the same thing over and over, the ipmitool is a great example of how to do it [1][2]\n\n [1] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipmitool.py#L104-L106\n\n [2] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipmitool.py#L140-L168","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"698242451b30c6afd4f43a4d86b00bcbbab2401b","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        output, err \u003d processutils.execute(\u0027mkisofs\u0027, \u0027--help\u0027)"},{"line_number":315,"context_line":"        is_supported \u003d False"},{"line_number":316,"context_line":"        if \u0027-e\u0027 or \u0027EFI\u0027 in err:"},{"line_number":317,"context_line":"                    is_supported \u003d True"},{"line_number":318,"context_line":"        else:"},{"line_number":319,"context_line":"            LOG.error(_LE(\"mkisofs does not support -e option\"))"},{"line_number":320,"context_line":"            raise exception.OptionNotSupported(_("}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_697a8625","line":317,"updated":"2014-09-11 08:40:48.000000000","message":"super nit: identation","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"}],"ironic/drivers/utils.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4a8ada9b70140ddaa9f64635b451ac8d741172f1","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    if capability is None:"},{"line_number":153,"context_line":"        return"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    if capabilities:"},{"line_number":156,"context_line":"        new_cap \u003d None"},{"line_number":157,"context_line":"        for cap in str(capabilities).split(\u0027,\u0027):"},{"line_number":158,"context_line":"            parts \u003d cap.split(\u0027:\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_d57a9824","line":155,"updated":"2014-09-08 09:27:40.000000000","message":"We can remove this conditional (if...else) here by changing the previous conditional (the one that test \"if capabilities is None\") with:\n\n if not capability:\n     return","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"da71e8f9be3adbb8357bc0b9081e29e0765bc7bc","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    if capability is None:"},{"line_number":153,"context_line":"        return"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    if capabilities:"},{"line_number":156,"context_line":"        new_cap \u003d None"},{"line_number":157,"context_line":"        for cap in str(capabilities).split(\u0027,\u0027):"},{"line_number":158,"context_line":"            parts \u003d cap.split(\u0027:\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_7cc9bf59","line":155,"in_reply_to":"fa98f980_d57a9824","updated":"2014-09-08 22:16:58.000000000","message":"done","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from ironic.common import exception"},{"line_number":16,"context_line":"from ironic.common import i18n"},{"line_number":17,"context_line":"from ironic.common.i18n import _"},{"line_number":18,"context_line":"from ironic.drivers import base"},{"line_number":19,"context_line":"from ironic.openstack.common import log as logging"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_dbe61144","line":16,"updated":"2014-09-09 17:59:17.000000000","message":"you don\u0027t need this.\n\nSee https://review.openstack.org/118587","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from ironic.drivers import base"},{"line_number":19,"context_line":"from ironic.openstack.common import log as logging"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"_LW \u003d i18n._LW"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_fbb3d54a","line":21,"updated":"2014-09-09 17:59:17.000000000","message":"This should be moved up after line 17, as \u0027from ironic.common.i18n import _LW\u0027\n\nSee https://review.openstack.org/118587.","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        if len(parts) \u003d\u003d 2 and parts[0] and parts[1]:"},{"line_number":162,"context_line":"            if parts[0] \u003d\u003d capability:"},{"line_number":163,"context_line":"                continue"},{"line_number":164,"context_line":"        new_cap \u003d new_cap + \u0027,\u0027 + cap if new_cap else cap"},{"line_number":165,"context_line":"    node.properties[\u0027capabilities\u0027] \u003d new_cap"},{"line_number":166,"context_line":"    node.save(task.context)"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_fb14b5ea","line":164,"updated":"2014-09-09 17:59:17.000000000","message":"How about having a cap_list\u003d[], then cap_list.append(cap) here, and at the end, set node.properties[\u0027capabilities\u0027] \u003d \u0027,\u0027.join(cap_list)\n\n(Not sure how true, I read that string concatenation was expensive)","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8e1ba6ae2f38ed59907b90de64b3ff9ec77bf9de","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    :param task: Task object."},{"line_number":173,"context_line":"    :param capability: Capability key."},{"line_number":174,"context_line":"    :param value: Capability value."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    \"\"\""},{"line_number":177,"context_line":"    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_7b8605dd","line":174,"updated":"2014-09-09 17:59:17.000000000","message":"So you\u0027re adding \u0027capability\u0027, without checking to see if it already exists. Is it OK to add the same capability key twice?","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cdc06251a25c798a0c0623331e6ae7f079dcf8cd","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    if not capabilities:"},{"line_number":129,"context_line":"        return"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    for node_capability in str(capabilities).split(\u0027,\u0027):"},{"line_number":132,"context_line":"        parts \u003d node_capability.split(\u0027:\u0027)"},{"line_number":133,"context_line":"        if len(parts) \u003d\u003d 2 and parts[0] and parts[1]:"},{"line_number":134,"context_line":"            if parts[0] \u003d\u003d capability:"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_83816591","line":131,"updated":"2014-09-10 13:21:59.000000000","message":"nit: what is this str() call for? e.g. you don\u0027t do it on line 179","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"33d0920bfc88650d353179509d93ab1647183764","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    capabilities \u003d node.properties.get(\u0027capabilities\u0027)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    if not capability:"},{"line_number":152,"context_line":"        return"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    if not capabilities:"},{"line_number":155,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_3aac65a0","line":152,"updated":"2014-09-10 09:39:52.000000000","message":"I don\u0027t think there\u0027s a case for that, unless we do something programatically wrong in the code \n\nThe only place that calls this function is in ilo/common.py:\n\n driver_utils.rm_node_capability(task, \u0027boot_mode\u0027)\n\nThe capability \u0027boot_mode\u0027 will be always there... As this is not exposed to any api I don\u0027t think we have a chance to call this method with a NULL parameter. And if we do it\u0027s better to leave it fail instead of returning None (which is the same value returned when the function succeed), this can hid bugs so I think we should get rid of this conditional\n\nps* sorry I wasn\u0027t so explicity on patch-set 7","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cdc06251a25c798a0c0623331e6ae7f079dcf8cd","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        return"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    new_cap \u003d None"},{"line_number":158,"context_line":"    for cap in str(capabilities).split(\u0027,\u0027):"},{"line_number":159,"context_line":"        parts \u003d cap.split(\u0027:\u0027)"},{"line_number":160,"context_line":"        if len(parts) \u003d\u003d 2 and parts[0] and parts[1]:"},{"line_number":161,"context_line":"            if parts[0] \u003d\u003d capability:"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_e34dd19a","line":158,"updated":"2014-09-10 13:21:59.000000000","message":"ditto about str","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ca5c85041754ddf3a98572a0ba1bc53a2f8fe027","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    :param task: Task object."},{"line_number":172,"context_line":"    :param capability: Capability key."},{"line_number":173,"context_line":"    :param value: Capability value."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    \"\"\""},{"line_number":176,"context_line":"    node \u003d task.node"},{"line_number":177,"context_line":"    capabilities \u003d node.properties.get(\u0027capabilities\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_da8b4b1c","line":174,"updated":"2014-09-10 16:50:34.000000000","message":"As a standalone method (ie, can\u0027t assume someone called rm_node_capability() first), is it OK to add a capability that is already there?\n\nIf it is OK, then fine.\n\nIf it is not OK, then something more needs to be done.","commit_id":"29582dbae2133c286473c68cc6d7bbda457a17e8"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9ba480643f7e2015fc4ee104fd92c5e9f961417d","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        if len(parts) \u003d\u003d 2 and parts[0] and parts[1]:"},{"line_number":158,"context_line":"            if parts[0] \u003d\u003d capability:"},{"line_number":159,"context_line":"                continue"},{"line_number":160,"context_line":"        new_cap \u003d new_cap + \u0027,\u0027 + cap if new_cap else cap"},{"line_number":161,"context_line":"    node.properties[\u0027capabilities\u0027] \u003d new_cap"},{"line_number":162,"context_line":"    node.save(task.context)"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_98b6123d","line":160,"updated":"2014-09-11 11:35:13.000000000","message":"yeah, caught you :D\n\nas you\u0027re anyway going to make another patch, please make sure to use \",\".join() instead of string concatenation. It\u0027s both suboptimal and antipattern.","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        if len(parts) \u003d\u003d 2 and parts[0] and parts[1]:"},{"line_number":158,"context_line":"            if parts[0] \u003d\u003d capability:"},{"line_number":159,"context_line":"                continue"},{"line_number":160,"context_line":"        new_cap \u003d new_cap + \u0027,\u0027 + cap if new_cap else cap"},{"line_number":161,"context_line":"    node.properties[\u0027capabilities\u0027] \u003d new_cap"},{"line_number":162,"context_line":"    node.save(task.context)"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_f3f49f5c","line":160,"in_reply_to":"fa98f980_98b6123d","updated":"2014-09-11 12:51:06.000000000","message":"Done.","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9ba480643f7e2015fc4ee104fd92c5e9f961417d","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"def add_node_capability(task, capability, value):"},{"line_number":166,"context_line":"    \"\"\"Add \u0027capability\u0027 to node\u0027s \u0027capabilities\u0027 property."},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    :param task: Task object."},{"line_number":169,"context_line":"    :param capability: Capability key."}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_78940e8b","line":166,"updated":"2014-09-11 11:35:13.000000000","message":"please describe what will happen on duplicate capability. It seems like it\u0027s ok - please state here","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"def add_node_capability(task, capability, value):"},{"line_number":166,"context_line":"    \"\"\"Add \u0027capability\u0027 to node\u0027s \u0027capabilities\u0027 property."},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    :param task: Task object."},{"line_number":169,"context_line":"    :param capability: Capability key."}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_d3ef9b85","line":166,"in_reply_to":"fa98f980_78940e8b","updated":"2014-09-11 12:51:06.000000000","message":"Done","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"38a896c0cb3e344dc210cd2f1f3cee6b1945cbc4","unresolved":false,"context_lines":[{"line_number":159,"context_line":"                continue"},{"line_number":160,"context_line":"        caps.append(cap)"},{"line_number":161,"context_line":"    new_cap_str \u003d \",\".join(caps)"},{"line_number":162,"context_line":"    node.properties[\u0027capabilities\u0027] \u003d new_cap_str if len(new_cap_str) else None"},{"line_number":163,"context_line":"    node.save(task.context)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_bca7384f","line":162,"updated":"2014-09-11 20:40:45.000000000","message":"Can just use `if new_cap_str`, will be False if it is an empty string","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":159,"context_line":"                continue"},{"line_number":160,"context_line":"        caps.append(cap)"},{"line_number":161,"context_line":"    new_cap_str \u003d \",\".join(caps)"},{"line_number":162,"context_line":"    node.properties[\u0027capabilities\u0027] \u003d new_cap_str if len(new_cap_str) else None"},{"line_number":163,"context_line":"    node.save(task.context)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_413820a0","line":162,"in_reply_to":"fa98f980_bca7384f","updated":"2014-09-11 21:32:55.000000000","message":"Yes, we can use just if new_cap_str.\n\nThis will be changed.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    node \u003d task.node"},{"line_number":178,"context_line":"    capabilities \u003d node.properties.get(\u0027capabilities\u0027)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d capabilities + \u0027,\u0027 + new_cap"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_f9063dcd","line":180,"updated":"2014-09-11 16:45:11.000000000","message":"please do new_cap \u003d \u0027:\u0027.join([capability, value])","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    node \u003d task.node"},{"line_number":178,"context_line":"    capabilities \u003d node.properties.get(\u0027capabilities\u0027)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d capabilities + \u0027,\u0027 + new_cap"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_6138c73d","line":180,"in_reply_to":"fa98f980_f9063dcd","updated":"2014-09-11 21:32:55.000000000","message":"Done","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"38a896c0cb3e344dc210cd2f1f3cee6b1945cbc4","unresolved":false,"context_lines":[{"line_number":180,"context_line":"    new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d capabilities + \u0027,\u0027 + new_cap"},{"line_number":184,"context_line":"    else:"},{"line_number":185,"context_line":"        capabilities \u003d new_cap"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_dca4fc4c","line":183,"updated":"2014-09-11 20:40:45.000000000","message":"Would prefer \u0027,\u0027.join((capabilities, new_cap)) here, but that might just be me.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":180,"context_line":"    new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d capabilities + \u0027,\u0027 + new_cap"},{"line_number":184,"context_line":"    else:"},{"line_number":185,"context_line":"        capabilities \u003d new_cap"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_393fe588","line":183,"updated":"2014-09-11 16:45:11.000000000","message":"please do capabilities \u003d \u0027,\u0027.join([capabilities, new_cap])","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":180,"context_line":"    new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d capabilities + \u0027,\u0027 + new_cap"},{"line_number":184,"context_line":"    else:"},{"line_number":185,"context_line":"        capabilities \u003d new_cap"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_01145b9e","line":183,"in_reply_to":"fa98f980_393fe588","updated":"2014-09-11 21:32:55.000000000","message":"Done","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":180,"context_line":"    new_cap \u003d capability + \u0027:\u0027 + value"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d capabilities + \u0027,\u0027 + new_cap"},{"line_number":184,"context_line":"    else:"},{"line_number":185,"context_line":"        capabilities \u003d new_cap"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_616444b9","line":183,"in_reply_to":"fa98f980_dca4fc4c","updated":"2014-09-11 21:32:55.000000000","message":"Done","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e7e59d09e9ae0d7614ddaa8980dbd02f1914e983","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    node \u003d task.node"},{"line_number":178,"context_line":"    capabilities \u003d node.properties.get(\u0027capabilities\u0027)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    new_cap \u003d \u0027:\u0027.join([capability, value])"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d \u0027,\u0027.join([capabilities, new_cap])"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_743eb98c","line":180,"updated":"2014-09-12 07:42:55.000000000","message":"It\u0027s a bit overdone here and below:\n\n new_cap \u003d \u0027%s:%s\u0027 % (capability, value)\n\nanyway please only change if you get -1 for something else (and make sure the others agree with me here)","commit_id":"d9cff4a2d829afcd98abbc35f12505f3b54dca92"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0b682b3453ab968fa9bd303fe22a245615dded39","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    node \u003d task.node"},{"line_number":178,"context_line":"    capabilities \u003d node.properties.get(\u0027capabilities\u0027)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    new_cap \u003d \u0027:\u0027.join([capability, value])"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    if capabilities:"},{"line_number":183,"context_line":"        capabilities \u003d \u0027,\u0027.join([capabilities, new_cap])"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_664364af","line":180,"in_reply_to":"fa98f980_743eb98c","updated":"2014-09-12 13:52:44.000000000","message":"+1\n\nyeah it looks like overkill :)","commit_id":"d9cff4a2d829afcd98abbc35f12505f3b54dca92"}],"ironic/tests/drivers/ilo/test_common.py":[{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"0846ee59cd18a364c4009cc395126729d78c73e0","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            set_node_capability_mock.assert_called_once_with(task,"},{"line_number":334,"context_line":"                                                             \u0027boot_mode\u0027,"},{"line_number":335,"context_line":"                                                             \u0027bios\u0027)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def test_set_node_capability_legacy(self):"},{"line_number":338,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":339,"context_line":"                                  shared\u003dFalse) as task:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_cc0d19ea","line":336,"updated":"2014-09-04 00:32:24.000000000","message":"There\u0027s a couple code paths in test_set_node_capability not tested here: given a list of existing capabilities, add a capability that isn\u0027t there, update a capability that already exists, and add a capability if node.capabilities is None/empty list (not sure what default is)","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"0846ee59cd18a364c4009cc395126729d78c73e0","unresolved":false,"context_lines":[{"line_number":337,"context_line":"    def test_set_node_capability_legacy(self):"},{"line_number":338,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":339,"context_line":"                                  shared\u003dFalse) as task:"},{"line_number":340,"context_line":"            ilo_common.set_node_capability(task, \u0027boot_mode\u0027, \u0027bios\u0027)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def test_get_node_capability(self):"},{"line_number":343,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_2cdce572","line":340,"updated":"2014-09-04 00:32:24.000000000","message":"This should test that the values that are set/not set are as expected.","commit_id":"3eded7e3e4db67e2fcef8c6b7be234b803b1f428"},{"author":{"_account_id":5805,"name":"Chris Krelle","email":"nobodycam@gmail.com","username":"nobodycam"},"change_message_id":"6ba72f7a175b208140c83db707cc40b61d06d0f6","unresolved":false,"context_lines":[{"line_number":371,"context_line":"        get_name_mock.return_value \u003d \u0027image-node-uuid\u0027"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":374,"context_line":"                                 shared\u003dFalse) as task:"},{"line_number":375,"context_line":"            ilo_common.cleanup_vmedia_boot(task)"},{"line_number":376,"context_line":"            swift_obj_mock.delete_object.assert_called_once_with(\u0027ilo_cont\u0027,"},{"line_number":377,"context_line":"                     \u0027image-node-uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa98f980_966e3a95","line":374,"updated":"2014-09-09 00:03:15.000000000","message":"why the spacing edit?","commit_id":"2fd5e875ac11bf64604d62a1ebf49fdb774447f2"},{"author":{"_account_id":5805,"name":"Chris Krelle","email":"nobodycam@gmail.com","username":"nobodycam"},"change_message_id":"6ba72f7a175b208140c83db707cc40b61d06d0f6","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                                 shared\u003dFalse) as task:"},{"line_number":375,"context_line":"            ilo_common.cleanup_vmedia_boot(task)"},{"line_number":376,"context_line":"            swift_obj_mock.delete_object.assert_called_once_with(\u0027ilo_cont\u0027,"},{"line_number":377,"context_line":"                     \u0027image-node-uuid\u0027)"},{"line_number":378,"context_line":"            ilo_object_mock.eject_virtual_media.assert_any_call(\u0027CDROM\u0027)"},{"line_number":379,"context_line":"            ilo_object_mock.eject_virtual_media.assert_any_call(\u0027FLOPPY\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa98f980_160a4ad2","line":377,"updated":"2014-09-09 00:03:15.000000000","message":"ditto","commit_id":"2fd5e875ac11bf64604d62a1ebf49fdb774447f2"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9ba480643f7e2015fc4ee104fd92c5e9f961417d","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"},{"line_number":269,"context_line":"    def test_set_boot_mode(self, get_ilo_object_mock):"},{"line_number":270,"context_line":"        ilo_object_mock \u003d mock.MagicMock()"},{"line_number":271,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_object_mock"},{"line_number":272,"context_line":"        get_pending_boot_mode_mock \u003d ilo_object_mock.get_pending_boot_mode"},{"line_number":273,"context_line":"        set_pending_boot_mode_mock \u003d ilo_object_mock.set_pending_boot_mode"},{"line_number":274,"context_line":"        get_pending_boot_mode_mock.return_value \u003d \u0027LEGACY\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_b8e1162c","line":271,"updated":"2014-09-11 11:35:13.000000000","message":"nit: these 2 lines equal to:\n\n ilo_object_mock \u003d get_ilo_object_mock.return_value","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"},{"line_number":269,"context_line":"    def test_set_boot_mode(self, get_ilo_object_mock):"},{"line_number":270,"context_line":"        ilo_object_mock \u003d mock.MagicMock()"},{"line_number":271,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_object_mock"},{"line_number":272,"context_line":"        get_pending_boot_mode_mock \u003d ilo_object_mock.get_pending_boot_mode"},{"line_number":273,"context_line":"        set_pending_boot_mode_mock \u003d ilo_object_mock.set_pending_boot_mode"},{"line_number":274,"context_line":"        get_pending_boot_mode_mock.return_value \u003d \u0027LEGACY\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_0e6c98cf","line":271,"in_reply_to":"fa98f980_b8e1162c","updated":"2014-09-11 12:51:06.000000000","message":"Done.","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            ilo_common.set_boot_mode(task.node, \u0027uefi\u0027)"},{"line_number":278,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":279,"context_line":"            get_pending_boot_mode_mock.assertEqual(\u0027LEGACY\u0027,"},{"line_number":280,"context_line":"                          get_pending_boot_mode_mock.return_value)"},{"line_number":281,"context_line":"            set_pending_boot_mode_mock.assert_called_once_with(\u0027UEFI\u0027)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_4e4ce042","line":280,"updated":"2014-09-11 12:51:06.000000000","message":"Fixed here also.\nget_pending_boot_mode_mock.assert_called_once_with(...)","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9ba480643f7e2015fc4ee104fd92c5e9f961417d","unresolved":false,"context_lines":[{"line_number":284,"context_line":"    def test_set_boot_mode_without_set_pending_boot_mode(self,"},{"line_number":285,"context_line":"                                                         get_ilo_object_mock):"},{"line_number":286,"context_line":"        ilo_object_mock \u003d mock.MagicMock()"},{"line_number":287,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_object_mock"},{"line_number":288,"context_line":"        get_pending_boot_mode_mock \u003d ilo_object_mock.get_pending_boot_mode"},{"line_number":289,"context_line":"        get_pending_boot_mode_mock.return_value \u003d \u0027LEGACY\u0027"},{"line_number":290,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_d8d17a58","line":287,"updated":"2014-09-11 11:35:13.000000000","message":"ditto","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":284,"context_line":"    def test_set_boot_mode_without_set_pending_boot_mode(self,"},{"line_number":285,"context_line":"                                                         get_ilo_object_mock):"},{"line_number":286,"context_line":"        ilo_object_mock \u003d mock.MagicMock()"},{"line_number":287,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_object_mock"},{"line_number":288,"context_line":"        get_pending_boot_mode_mock \u003d ilo_object_mock.get_pending_boot_mode"},{"line_number":289,"context_line":"        get_pending_boot_mode_mock.return_value \u003d \u0027LEGACY\u0027"},{"line_number":290,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_2e715ca8","line":287,"in_reply_to":"fa98f980_d8d17a58","updated":"2014-09-11 12:51:06.000000000","message":"Done","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9ba480643f7e2015fc4ee104fd92c5e9f961417d","unresolved":false,"context_lines":[{"line_number":292,"context_line":"            ilo_common.set_boot_mode(task.node, \u0027bios\u0027)"},{"line_number":293,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":294,"context_line":"            get_pending_boot_mode_mock.assertEqual(\u0027LEGACY\u0027,"},{"line_number":295,"context_line":"                          get_pending_boot_mode_mock.return_value)"},{"line_number":296,"context_line":"            ilo_object_mock.set_pending_boot_mode.assert_not_called()"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_985292dc","line":295,"updated":"2014-09-11 11:35:13.000000000","message":"this line is not testing anything and probably should be replaced with:\n\n get_pending_boot_mode_mock.assert_called_once_with(...)","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":292,"context_line":"            ilo_common.set_boot_mode(task.node, \u0027bios\u0027)"},{"line_number":293,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":294,"context_line":"            get_pending_boot_mode_mock.assertEqual(\u0027LEGACY\u0027,"},{"line_number":295,"context_line":"                          get_pending_boot_mode_mock.return_value)"},{"line_number":296,"context_line":"            ilo_object_mock.set_pending_boot_mode.assert_not_called()"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_aeb8ec11","line":295,"in_reply_to":"fa98f980_985292dc","updated":"2014-09-11 12:51:06.000000000","message":"Done.","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9ba480643f7e2015fc4ee104fd92c5e9f961417d","unresolved":false,"context_lines":[{"line_number":303,"context_line":"                                         add_node_capability_mock):"},{"line_number":304,"context_line":"        ilo_client_mock.IloCommandNotSupportedError \u003d Exception"},{"line_number":305,"context_line":"        ilo_mock_obj \u003d mock.MagicMock()"},{"line_number":306,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_mock_obj"},{"line_number":307,"context_line":"        ilo_mock_obj.get_pending_boot_mode.return_value \u003d \u0027legacy\u0027"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_f8d63e4e","line":306,"updated":"2014-09-11 11:35:13.000000000","message":"ditto","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":303,"context_line":"                                         add_node_capability_mock):"},{"line_number":304,"context_line":"        ilo_client_mock.IloCommandNotSupportedError \u003d Exception"},{"line_number":305,"context_line":"        ilo_mock_obj \u003d mock.MagicMock()"},{"line_number":306,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_mock_obj"},{"line_number":307,"context_line":"        ilo_mock_obj.get_pending_boot_mode.return_value \u003d \u0027legacy\u0027"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_ce38d0a2","line":306,"in_reply_to":"fa98f980_f8d63e4e","updated":"2014-09-11 12:51:06.000000000","message":"Done","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9ba480643f7e2015fc4ee104fd92c5e9f961417d","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                                                add_node_capability_mock):"},{"line_number":324,"context_line":"        ilo_client_mock.IloCommandNotSupportedError \u003d Exception"},{"line_number":325,"context_line":"        ilo_mock_obj \u003d mock.MagicMock()"},{"line_number":326,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_mock_obj"},{"line_number":327,"context_line":"        ilo_mock_obj.get_pending_boot_mode.side_effect \u003d Exception"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_58c58a8c","line":326,"updated":"2014-09-11 11:35:13.000000000","message":"ditto","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"861607ff19c02f47b6ef764ea630753ba8f20d26","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                                                add_node_capability_mock):"},{"line_number":324,"context_line":"        ilo_client_mock.IloCommandNotSupportedError \u003d Exception"},{"line_number":325,"context_line":"        ilo_mock_obj \u003d mock.MagicMock()"},{"line_number":326,"context_line":"        get_ilo_object_mock.return_value \u003d ilo_mock_obj"},{"line_number":327,"context_line":"        ilo_mock_obj.get_pending_boot_mode.side_effect \u003d Exception"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_ee3d9490","line":326,"in_reply_to":"fa98f980_58c58a8c","updated":"2014-09-11 12:51:06.000000000","message":"Done","commit_id":"4fc1c3464f27b02ea5f99e11ab35c27b6e749e36"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            ilo_common.set_boot_mode(task.node, \u0027uefi\u0027)"},{"line_number":276,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":277,"context_line":"            get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":278,"context_line":"            set_pending_boot_mode_mock.assert_called_once_with(\u0027UEFI\u0027)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"},{"line_number":281,"context_line":"    def test_set_boot_mode_without_set_pending_boot_mode(self,"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_ffa1bdf8","line":278,"updated":"2014-09-11 16:45:11.000000000","message":"Wondering. Why do you need the task? Can\u0027t you just use self.node for the test?","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            ilo_common.set_boot_mode(task.node, \u0027uefi\u0027)"},{"line_number":276,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":277,"context_line":"            get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":278,"context_line":"            set_pending_boot_mode_mock.assert_called_once_with(\u0027UEFI\u0027)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"},{"line_number":281,"context_line":"    def test_set_boot_mode_without_set_pending_boot_mode(self,"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_c1f9734f","line":278,"in_reply_to":"fa98f980_ffa1bdf8","updated":"2014-09-11 21:32:55.000000000","message":"Yes, not required. \nChanged the testcase to use self.node","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":288,"context_line":"            ilo_common.set_boot_mode(task.node, \u0027bios\u0027)"},{"line_number":289,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":290,"context_line":"            get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":291,"context_line":"            ilo_object_mock.set_pending_boot_mode.assert_not_called()"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"},{"line_number":294,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_3faee598","line":291,"updated":"2014-09-11 16:45:11.000000000","message":"assert_not_called() isn\u0027t a valid mock method\n\nI think you want to do something like\n    self.assertFalse(ilo_object_mock.set_pending_boot_mode.called)","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":288,"context_line":"            ilo_common.set_boot_mode(task.node, \u0027bios\u0027)"},{"line_number":289,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":290,"context_line":"            get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":291,"context_line":"            ilo_object_mock.set_pending_boot_mode.assert_not_called()"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"},{"line_number":294,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_41102359","line":291,"in_reply_to":"fa98f980_3faee598","updated":"2014-09-11 21:32:55.000000000","message":"Done","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":289,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":290,"context_line":"            get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":291,"context_line":"            ilo_object_mock.set_pending_boot_mode.assert_not_called()"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"},{"line_number":294,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"},{"line_number":295,"context_line":"    @mock.patch.object(ilo_common, \u0027ilo_client\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_3f5d057a","line":292,"updated":"2014-09-11 16:45:11.000000000","message":"It might be nice to see a test where exception.IloOperationError is raised by set_boot_mode().","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":289,"context_line":"            get_ilo_object_mock.assert_called_once_with(task.node)"},{"line_number":290,"context_line":"            get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":291,"context_line":"            ilo_object_mock.set_pending_boot_mode.assert_not_called()"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"},{"line_number":294,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"},{"line_number":295,"context_line":"    @mock.patch.object(ilo_common, \u0027ilo_client\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_9eb269e3","line":292,"in_reply_to":"fa98f980_3f5d057a","updated":"2014-09-11 21:32:55.000000000","message":"Added a new test case","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":307,"context_line":"            ilo_mock_obj.get_pending_boot_mode.assert_called_once_with()"},{"line_number":308,"context_line":"            add_node_capability_mock.assert_called_once_with(task,"},{"line_number":309,"context_line":"                                                             \u0027boot_mode\u0027,"},{"line_number":310,"context_line":"                                                             \u0027bios\u0027)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"},{"line_number":313,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_1f3921f2","line":310,"updated":"2014-09-11 16:45:11.000000000","message":"It seems a bit odd that you aren\u0027t mocking driver_utils.rm_node_capability, but I guess it is fine?","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":307,"context_line":"            ilo_mock_obj.get_pending_boot_mode.assert_called_once_with()"},{"line_number":308,"context_line":"            add_node_capability_mock.assert_called_once_with(task,"},{"line_number":309,"context_line":"                                                             \u0027boot_mode\u0027,"},{"line_number":310,"context_line":"                                                             \u0027bios\u0027)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    @mock.patch.object(driver_utils, \u0027add_node_capability\u0027)"},{"line_number":313,"context_line":"    @mock.patch.object(ilo_common, \u0027get_ilo_object\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_3e133db9","line":310,"in_reply_to":"fa98f980_1f3921f2","updated":"2014-09-11 21:32:55.000000000","message":"Added a mock for rm_node_capability","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            add_node_capability_mock.assert_called_once_with(task,"},{"line_number":349,"context_line":"                                                             \u0027boot_mode\u0027,"},{"line_number":350,"context_line":"                                                             \u0027bios\u0027)"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    @mock.patch.object(images, \u0027get_temp_url_for_glance_image\u0027)"},{"line_number":353,"context_line":"    @mock.patch.object(ilo_common, \u0027attach_vmedia\u0027)"},{"line_number":354,"context_line":"    @mock.patch.object(ilo_common, \u0027_prepare_floppy_image\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_45a836db","line":351,"updated":"2014-09-11 16:45:11.000000000","message":"I haven\u0027t looked at all our tests to see how they all handle this. I\u0027m basing my comments on how we handle ipminative tests.\n\nThe ilo_client stuff can (should?) be handled in ../third_party_driver_mocks.py, so that you shouldn\u0027t need to mock that here in the individual tests. In the other file, do something like:\n\n proliantutils \u003d importutils.try_import(\u0027proliantutils\u0027)\n if not proliantutils:\n     mock_proliant_utils \u003d mock.MagicMock()\n     mock_proliant_utils.ilo \u003d mock.MagicMock()\n     mock_proliant_utils.ilo.ribcl \u003d mock.MagicMock()\n     mock_proliant_utils.ilo.ribcl.IloCommandNotSupportedError \u003d Exception\n     sys.modules[\u0027proliantutils\u0027] \u003d mock_proliant_utils\n     sys.modules[\u0027proliantutils.ilo\u0027] \u003d mock_proliant_utils.ilo\n     sys.modules[\u0027proliantutils.ilo.ribcl\u0027] \u003d mock_proliant_utils.ilo.ribcl\n\n[or maybe this can be done in a separate patch, dunno.]","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            add_node_capability_mock.assert_called_once_with(task,"},{"line_number":349,"context_line":"                                                             \u0027boot_mode\u0027,"},{"line_number":350,"context_line":"                                                             \u0027bios\u0027)"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    @mock.patch.object(images, \u0027get_temp_url_for_glance_image\u0027)"},{"line_number":353,"context_line":"    @mock.patch.object(ilo_common, \u0027attach_vmedia\u0027)"},{"line_number":354,"context_line":"    @mock.patch.object(ilo_common, \u0027_prepare_floppy_image\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_de231103","line":351,"in_reply_to":"fa98f980_45a836db","updated":"2014-09-11 21:32:55.000000000","message":"This may require changes to existing test cases as well.\nWe can address this in another patch later.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d3989a8e21428673bcb14dcc19c4c27f6d49adb7","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        ilo_client_mock.IloError \u003d Exception"},{"line_number":299,"context_line":"        set_pending_boot_mode_mock.side_effect \u003d Exception"},{"line_number":300,"context_line":"        self.assertRaises(exception.IloOperationError,"},{"line_number":301,"context_line":"                        ilo_common.set_boot_mode, self.node, \u0027bios\u0027)"},{"line_number":302,"context_line":"        get_ilo_object_mock.assert_called_once_with(self.node)"},{"line_number":303,"context_line":"        get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":304,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_c4f97ec9","line":301,"updated":"2014-09-11 22:05:34.000000000","message":"it looks like there might be something off with the alignment; should this be aligned under \u0027exception...\u0027?","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"},{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"6c47691a3d5284135ad53b330e0e442b117913dc","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        ilo_client_mock.IloError \u003d Exception"},{"line_number":299,"context_line":"        set_pending_boot_mode_mock.side_effect \u003d Exception"},{"line_number":300,"context_line":"        self.assertRaises(exception.IloOperationError,"},{"line_number":301,"context_line":"                        ilo_common.set_boot_mode, self.node, \u0027bios\u0027)"},{"line_number":302,"context_line":"        get_ilo_object_mock.assert_called_once_with(self.node)"},{"line_number":303,"context_line":"        get_pending_boot_mode_mock.assert_called_once_with()"},{"line_number":304,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_68e420e0","line":301,"in_reply_to":"fa98f980_c4f97ec9","updated":"2014-09-12 04:42:04.000000000","message":"Ok","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"}],"ironic/tests/drivers/ilo/test_deploy.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":93,"context_line":"            deploy_info_mock.assert_called_once_with(task.node)"},{"line_number":94,"context_line":"            image_prop_mock.assert_called_once_with(task.context, \u0027image-uuid\u0027,"},{"line_number":95,"context_line":"                \u0027boot_iso\u0027)"},{"line_number":96,"context_line":"            self.assertIsNone(boot_iso_result)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    @mock.patch.object(tempfile, \u0027NamedTemporaryFile\u0027)"},{"line_number":99,"context_line":"    @mock.patch.object(images, \u0027create_boot_iso\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_253fb288","line":96,"updated":"2014-09-11 16:45:11.000000000","message":"maybe also check that get_node_cap_mock was called?","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":93,"context_line":"            deploy_info_mock.assert_called_once_with(task.node)"},{"line_number":94,"context_line":"            image_prop_mock.assert_called_once_with(task.context, \u0027image-uuid\u0027,"},{"line_number":95,"context_line":"                \u0027boot_iso\u0027)"},{"line_number":96,"context_line":"            self.assertIsNone(boot_iso_result)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    @mock.patch.object(tempfile, \u0027NamedTemporaryFile\u0027)"},{"line_number":99,"context_line":"    @mock.patch.object(images, \u0027create_boot_iso\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_3eefbd88","line":96,"in_reply_to":"fa98f980_253fb288","updated":"2014-09-11 21:32:55.000000000","message":"Done","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"}],"ironic/tests/drivers/test_utils.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4a8ada9b70140ddaa9f64635b451ac8d741172f1","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b\u0027"},{"line_number":168,"context_line":"            driver_utils.rm_node_capability(task, \u0027a\u0027)"},{"line_number":169,"context_line":"            self.assertIsNone(task.node.properties[\u0027capabilities\u0027])"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def test_validate_boot_mode_capability(self):"},{"line_number":172,"context_line":"        properties \u003d {\u0027capabilities\u0027: \u0027boot_mode:uefi,cap2:value2\u0027}"},{"line_number":173,"context_line":"        self.node.properties \u003d properties"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_159260f6","line":170,"updated":"2014-09-08 09:27:40.000000000","message":"def test_rm_node_capability_non_existent(self)\n\n to check whether it\u0027s going to return None or fail","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"da71e8f9be3adbb8357bc0b9081e29e0765bc7bc","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b\u0027"},{"line_number":168,"context_line":"            driver_utils.rm_node_capability(task, \u0027a\u0027)"},{"line_number":169,"context_line":"            self.assertIsNone(task.node.properties[\u0027capabilities\u0027])"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def test_validate_boot_mode_capability(self):"},{"line_number":172,"context_line":"        properties \u003d {\u0027capabilities\u0027: \u0027boot_mode:uefi,cap2:value2\u0027}"},{"line_number":173,"context_line":"        self.node.properties \u003d properties"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_fc5f6f73","line":170,"in_reply_to":"fa98f980_159260f6","updated":"2014-09-08 22:16:58.000000000","message":"Added a new test case.","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b,c:d\u0027"},{"line_number":160,"context_line":"            driver_utils.add_node_capability(task, \u0027boot_mode\u0027, \u0027bios\u0027)"},{"line_number":161,"context_line":"            self.assertEqual(\u0027a:b,c:d,boot_mode:bios\u0027,"},{"line_number":162,"context_line":"                              task.node.properties[\u0027capabilities\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def test_rm_node_capability(self):"},{"line_number":165,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_4538363d","line":162,"updated":"2014-09-11 16:45:11.000000000","message":"I\u0027d also like to see a test that adds a key:value to capabilities that already has the same key.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b,c:d\u0027"},{"line_number":160,"context_line":"            driver_utils.add_node_capability(task, \u0027boot_mode\u0027, \u0027bios\u0027)"},{"line_number":161,"context_line":"            self.assertEqual(\u0027a:b,c:d,boot_mode:bios\u0027,"},{"line_number":162,"context_line":"                              task.node.properties[\u0027capabilities\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def test_rm_node_capability(self):"},{"line_number":165,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_1e8d99d3","line":162,"in_reply_to":"fa98f980_4538363d","updated":"2014-09-11 21:32:55.000000000","message":"Added a test case, that adds a duplicate entry in the capabilities property","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":173,"context_line":"                                  shared\u003dFalse) as task:"},{"line_number":174,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b\u0027"},{"line_number":175,"context_line":"            self.assertIsNone(driver_utils.rm_node_capability(task, \u0027x\u0027))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def test_validate_boot_mode_capability(self):"},{"line_number":178,"context_line":"        properties \u003d {\u0027capabilities\u0027: \u0027boot_mode:uefi,cap2:value2\u0027}"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_a5c1e230","line":175,"updated":"2014-09-11 16:45:11.000000000","message":"doesn\u0027t driver_utils.rm_node_capability() return None even if capabilities is None? (Ie, I see two places where it can return None.)","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":173,"context_line":"                                  shared\u003dFalse) as task:"},{"line_number":174,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b\u0027"},{"line_number":175,"context_line":"            self.assertIsNone(driver_utils.rm_node_capability(task, \u0027x\u0027))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def test_validate_boot_mode_capability(self):"},{"line_number":178,"context_line":"        properties \u003d {\u0027capabilities\u0027: \u0027boot_mode:uefi,cap2:value2\u0027}"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_414e6070","line":175,"in_reply_to":"fa98f980_a5c1e230","updated":"2014-09-11 21:32:55.000000000","message":"Actually we should check that the capabilities is not modified if rm_node_capability() try to remove the capability which is not present.\n\nAdded this check.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea334ecc715a437e01349b4bbee6fc7d18b210d8","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                  shared\u003dFalse) as task:"},{"line_number":174,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b\u0027"},{"line_number":175,"context_line":"            self.assertIsNone(driver_utils.rm_node_capability(task, \u0027x\u0027))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def test_validate_boot_mode_capability(self):"},{"line_number":178,"context_line":"        properties \u003d {\u0027capabilities\u0027: \u0027boot_mode:uefi,cap2:value2\u0027}"},{"line_number":179,"context_line":"        self.node.properties \u003d properties"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_85f69e8d","line":176,"updated":"2014-09-11 16:45:11.000000000","message":"I\u0027d also like to see a test for rm_node_capability where capabilities has eg 3 key:value pairs, and you remove one of them. This is to make sure the removal doesn\u0027t mess up the remaining two.","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"fa4f4b01d1291ad5c5abc22cba91f2feb04ddd22","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                  shared\u003dFalse) as task:"},{"line_number":174,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b\u0027"},{"line_number":175,"context_line":"            self.assertIsNone(driver_utils.rm_node_capability(task, \u0027x\u0027))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def test_validate_boot_mode_capability(self):"},{"line_number":178,"context_line":"        properties \u003d {\u0027capabilities\u0027: \u0027boot_mode:uefi,cap2:value2\u0027}"},{"line_number":179,"context_line":"        self.node.properties \u003d properties"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa98f980_419ca0c0","line":176,"in_reply_to":"fa98f980_85f69e8d","updated":"2014-09-11 21:32:55.000000000","message":"Added a test case to cover this scenario","commit_id":"eaf49419cb1e29a8c4ab3350103a84f29522a402"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d3989a8e21428673bcb14dcc19c4c27f6d49adb7","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b,c:d\u0027"},{"line_number":168,"context_line":"            driver_utils.add_node_capability(task, \u0027a\u0027, \u0027b\u0027)"},{"line_number":169,"context_line":"            self.assertEqual(\u0027a:b,c:d,a:b\u0027,"},{"line_number":170,"context_line":"                              task.node.properties[\u0027capabilities\u0027])"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def test_rm_node_capability(self):"},{"line_number":173,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_04c5465b","line":170,"updated":"2014-09-11 22:05:34.000000000","message":"the alignment if off here too. One space too much.","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"},{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"6c47691a3d5284135ad53b330e0e442b117913dc","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            task.node.properties[\u0027capabilities\u0027] \u003d \u0027a:b,c:d\u0027"},{"line_number":168,"context_line":"            driver_utils.add_node_capability(task, \u0027a\u0027, \u0027b\u0027)"},{"line_number":169,"context_line":"            self.assertEqual(\u0027a:b,c:d,a:b\u0027,"},{"line_number":170,"context_line":"                              task.node.properties[\u0027capabilities\u0027])"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def test_rm_node_capability(self):"},{"line_number":173,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_a8d1a8fc","line":170,"in_reply_to":"fa98f980_04c5465b","updated":"2014-09-12 04:42:04.000000000","message":"ok","commit_id":"0a3756c40d28a9709cd70509ad9c8ddedd6a5a5b"}],"ironic/tests/test_images.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4a8ada9b70140ddaa9f64635b451ac8d741172f1","unresolved":false,"context_lines":[{"line_number":16,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":17,"context_line":"#    under the License."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"import fixtures"},{"line_number":20,"context_line":"import mock"},{"line_number":21,"context_line":"import os"},{"line_number":22,"context_line":"import shutil"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_d52378bc","line":19,"updated":"2014-09-08 09:27:40.000000000","message":"we got some patches to remove fixtures from our tests and use mock instead\n\n https://review.openstack.org/#/c/52155/\n https://review.openstack.org/#/c/79702/","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4a8ada9b70140ddaa9f64635b451ac8d741172f1","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            return size"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027os.path.getsize\u0027, fake_getsize))"},{"line_number":381,"context_line":"        efi_image_size \u003d (1000 + 1000 + 1000 + 1000) / 1024 + 4"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        images.create_isolinux_image(\u0027tgt_file\u0027, \u0027path/to/kernel\u0027,"},{"line_number":384,"context_line":"                \u0027path/to/ramdisk\u0027, kernel_params\u003dparams)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_751c8c78","line":381,"updated":"2014-09-08 09:27:40.000000000","message":"Let\u0027s use mock here please, getsize() is already being mocked. I believe you can do it like:\n\n getsize_mock \u003d mock.Mock(return_value\u003defi_image_size)","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"da71e8f9be3adbb8357bc0b9081e29e0765bc7bc","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            return size"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027os.path.getsize\u0027, fake_getsize))"},{"line_number":381,"context_line":"        efi_image_size \u003d (1000 + 1000 + 1000 + 1000) / 1024 + 4"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        images.create_isolinux_image(\u0027tgt_file\u0027, \u0027path/to/kernel\u0027,"},{"line_number":384,"context_line":"                \u0027path/to/ramdisk\u0027, kernel_params\u003dparams)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_455a5a84","line":381,"in_reply_to":"fa98f980_751c8c78","updated":"2014-09-08 22:16:58.000000000","message":"done","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4a8ada9b70140ddaa9f64635b451ac8d741172f1","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        efi_image_size \u003d (1000 + 1000 + 1000 + 1000) / 1024 + 4"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        images.create_isolinux_image(\u0027tgt_file\u0027, \u0027path/to/kernel\u0027,"},{"line_number":384,"context_line":"                \u0027path/to/ramdisk\u0027, kernel_params\u003dparams)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        create_root_fs_mock.assert_called_with(\u0027tmpdir\u0027, files_info)"},{"line_number":387,"context_line":"        gen_cfg_mock.assert_called_once_with(params)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_e7728fac","line":384,"updated":"2014-09-08 09:27:40.000000000","message":"this test is failing with:\n\n OSError: [Errno 2] No such file or directory: \u0027path/to/kernel\u0027\n\nThis is being caused by the os.stat(filename).st_size we probably have to mock os.stat as well","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":10574,"name":"Syed Ismail Faizan Barmawer","email":"faizan.barmawer@gmail.com","username":"barmawer"},"change_message_id":"da71e8f9be3adbb8357bc0b9081e29e0765bc7bc","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        efi_image_size \u003d (1000 + 1000 + 1000 + 1000) / 1024 + 4"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        images.create_isolinux_image(\u0027tgt_file\u0027, \u0027path/to/kernel\u0027,"},{"line_number":384,"context_line":"                \u0027path/to/ramdisk\u0027, kernel_params\u003dparams)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        create_root_fs_mock.assert_called_with(\u0027tmpdir\u0027, files_info)"},{"line_number":387,"context_line":"        gen_cfg_mock.assert_called_once_with(params)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_c54d6ac6","line":384,"in_reply_to":"fa98f980_e7728fac","updated":"2014-09-08 22:16:58.000000000","message":"not required, It got solved with getsize_mock.sideeffects \u003d []","commit_id":"e65d7b20df5cf23cbd3368cb54fbf36d7921c810"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b668e071e09b0c6c10555205cd2e3033c8dbabda","unresolved":false,"context_lines":[{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        getsize_mock.side_effect \u003d [1024, 1024, 1024, 1024]"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"        efi_image_size \u003d ((1024 * 4) + (1024 * 4 * 2 / 100)) / 1024"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        images.create_isolinux_image(\u0027tgt_file\u0027, \u0027path/to/kernel\u0027,"},{"line_number":380,"context_line":"                \u0027path/to/ramdisk\u0027, kernel_params\u003dparams)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_edc08ce2","line":377,"updated":"2014-09-09 07:44:05.000000000","message":"ditto about floats and rounding. As you use 1024 as size, you aren\u0027t affected by rounding, for smth like 2000, you will be","commit_id":"daf91f8ef0bcf1b478fc3563778ef2d363607937"}]}