)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"658048f514220819322a7f2783e0d05f545febf4","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The imp module is deprecated since Python 3.4[1] in favor of the new"},{"line_number":10,"context_line":"importlib module. This change replaces usage of that deprecated module."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1] https://docs.python.org/3/library/imp.html"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I798f345e729ae974917872ba1f2e43205a660c45"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"1f48e17a_11e0eeed","line":11,"updated":"2022-01-21 07:59:25.000000000","message":"This commit now cleans up the existing logic. It no longer returns sys.path[0] and raises an error when non-existing module name is specified in DJANGO_SETTINGS_MODULE. I think it is worth covering it in the commit message.\n\nCould you add a short description to the commit message?\n\nMy idea is:\n\n  Discussion in the review turns out that there is no need to return sys.path[0]\n  when an non-existing module name is passed to get_module_path()\n  as the returned value is just to used to find \"local\" directory for the\n  dashboard configurattion. Considering this, thie commit cleans up the logic\n  and the function raises an exception for such case now.\n\nThought?","commit_id":"ebcf1e36ab7878e9db466b334b3c20e32e3d9807"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"afb096040f64234a33f09fc8cb8ac63128555a6f","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The imp module is deprecated since Python 3.4[1] in favor of the new"},{"line_number":10,"context_line":"importlib module. This change replaces usage of that deprecated module."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1] https://docs.python.org/3/library/imp.html"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I798f345e729ae974917872ba1f2e43205a660c45"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"dc802cf7_b866cca8","line":11,"in_reply_to":"1f48e17a_11e0eeed","updated":"2022-01-24 08:36:49.000000000","message":"I spent some time on thinking of this and I\u0027ve updated the commit message to include that point as well as the removal of the loop logic. WDYT about this ?","commit_id":"ebcf1e36ab7878e9db466b334b3c20e32e3d9807"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"fc1cf581ea4179ccd362d24eb1eed0acefae58d3","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The imp module is deprecated since Python 3.4[1] in favor of the new"},{"line_number":10,"context_line":"importlib module. This change replaces usage of that deprecated module."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1] https://docs.python.org/3/library/imp.html"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I798f345e729ae974917872ba1f2e43205a660c45"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"a62c593a_6903c26c","line":11,"in_reply_to":"8203a478_ba4022d9","updated":"2022-02-08 13:58:54.000000000","message":"no problem :)\nIt is worth fixing it.","commit_id":"ebcf1e36ab7878e9db466b334b3c20e32e3d9807"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"b2599d486dd35122f17a8ee3bf5528e86bf3f070","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The imp module is deprecated since Python 3.4[1] in favor of the new"},{"line_number":10,"context_line":"importlib module. This change replaces usage of that deprecated module."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1] https://docs.python.org/3/library/imp.html"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I798f345e729ae974917872ba1f2e43205a660c45"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"8203a478_ba4022d9","line":11,"in_reply_to":"9ed192c0_5cbc0f9e","updated":"2022-02-08 13:16:17.000000000","message":"Thanks for checking. Just found one typo (\u0027if\u0027 instead of \u0027of\u0027) which might be confusing for future reference... sorry for making the nit change after +2 .","commit_id":"ebcf1e36ab7878e9db466b334b3c20e32e3d9807"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2e8b400d2f6be4d56c8b1d2b3236ad86f66504d6","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The imp module is deprecated since Python 3.4[1] in favor of the new"},{"line_number":10,"context_line":"importlib module. This change replaces usage of that deprecated module."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1] https://docs.python.org/3/library/imp.html"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I798f345e729ae974917872ba1f2e43205a660c45"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9ed192c0_5cbc0f9e","line":11,"in_reply_to":"dc802cf7_b866cca8","updated":"2022-02-08 13:09:52.000000000","message":"Thanks. The commit message sounds reasonable now 😊","commit_id":"ebcf1e36ab7878e9db466b334b3c20e32e3d9807"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":29313,"name":"Vishal Manchanda","email":"manchandavishal143@gmail.com","username":"vishalmanchanda"},"change_message_id":"be8acf0a6d9bf036b442d9c50fee9fdacf603aa1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c575ec85_1a9adfec","updated":"2022-01-10 06:16:41.000000000","message":"LGTM","commit_id":"dbf19448b2ff38890f9884a2ab7a2b2c6603d8b6"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"b7671630b027cb7cb34144e287975e78a340d353","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"87634da4_00b810fc","updated":"2022-01-18 07:12:41.000000000","message":"I noticed I still misunderstand the current implementation. Let me recheck it again...","commit_id":"3963f58b8bf597a30c3f95d286f95d4cc249b189"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"90e4d9b6cb8099d8f7ee05756b893a0d03075afc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"010d161e_f4e2fc61","updated":"2022-01-20 12:30:13.000000000","message":"recheck","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":29313,"name":"Vishal Manchanda","email":"manchandavishal143@gmail.com","username":"vishalmanchanda"},"change_message_id":"a4bc9407ed56182e9c429f1b5232386c1a0ad827","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"953dcec0_3ec275d5","updated":"2022-02-07 18:28:05.000000000","message":"recheck","commit_id":"eb92d16402e3c6e842d1df7adc74f3f0eeb18b2e"},{"author":{"_account_id":29313,"name":"Vishal Manchanda","email":"manchandavishal143@gmail.com","username":"vishalmanchanda"},"change_message_id":"5cf8cbacb2969cd2fbcec4b817c54329e1c990a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"e0facf9d_89f58534","updated":"2022-01-28 16:15:48.000000000","message":"recheck","commit_id":"eb92d16402e3c6e842d1df7adc74f3f0eeb18b2e"},{"author":{"_account_id":29313,"name":"Vishal Manchanda","email":"manchandavishal143@gmail.com","username":"vishalmanchanda"},"change_message_id":"c39e1a962e5c044a85015879c9c8d87ef53960a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"65ee2f3e_6f98c8dd","updated":"2022-02-09 04:28:53.000000000","message":"recheck","commit_id":"5d0b2b74d4fb62049ddcc2e4000c7771e0749165"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"5f9fc76e8d820b3da7bb0078b6474cf4e0cb0735","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"6d9c0f5c_c3d9b011","updated":"2022-02-09 11:56:18.000000000","message":"recheck","commit_id":"5d0b2b74d4fb62049ddcc2e4000c7771e0749165"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"372302a9bebf71435d2f1894af79559333996945","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"8d001cae_a362a1d0","updated":"2022-02-08 22:18:04.000000000","message":"recheck","commit_id":"5d0b2b74d4fb62049ddcc2e4000c7771e0749165"}],"openstack_dashboard/management/commands/migrate_settings.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"14fa72b7ff5d23a2cb2ed8e0e1293ecdd601169c","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":"    path \u003d sys.path"},{"line_number":36,"context_line":"    for name in module_name.split(\u0027.\u0027):"},{"line_number":37,"context_line":"        spec \u003d importlib.util.find_spec(name, path)"},{"line_number":38,"context_line":"        path \u003d [spec.origin, ]"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"b66a9990_b5af19b1","line":37,"range":{"start_line":37,"start_character":46,"end_line":37,"end_character":50},"updated":"2022-01-18 04:33:54.000000000","message":"sys.path is a list of directories. Looking at the importlib document [1], it looks like that \u0027path\u0027 is not a list of paths but a path. Is this the right usage of find_spec()?\n\nFor further discussion, see my comment in the next line.\n\n[1] https://docs.python.org/3/library/importlib.html#importlib.abc.MetaPathFinder.find_spec","commit_id":"dbf19448b2ff38890f9884a2ab7a2b2c6603d8b6"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"532615582aef9eb561dd3a81fc4c97c4ea306841","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":"    path \u003d sys.path"},{"line_number":36,"context_line":"    for name in module_name.split(\u0027.\u0027):"},{"line_number":37,"context_line":"        spec \u003d importlib.util.find_spec(name, path)"},{"line_number":38,"context_line":"        path \u003d [spec.origin, ]"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"8ef4906f_65e6add9","line":37,"range":{"start_line":37,"start_character":8,"end_line":37,"end_character":12},"updated":"2022-01-18 07:10:57.000000000","message":"we should consider the case when the spec is not found.","commit_id":"dbf19448b2ff38890f9884a2ab7a2b2c6603d8b6"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"532615582aef9eb561dd3a81fc4c97c4ea306841","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":"    path \u003d sys.path"},{"line_number":36,"context_line":"    for name in module_name.split(\u0027.\u0027):"},{"line_number":37,"context_line":"        spec \u003d importlib.util.find_spec(name, path)"},{"line_number":38,"context_line":"        path \u003d [spec.origin, ]"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"f6b18865_9735bc31","line":37,"range":{"start_line":37,"start_character":46,"end_line":37,"end_character":50},"in_reply_to":"b66a9990_b5af19b1","updated":"2022-01-18 07:10:57.000000000","message":"You are correct and the argument should be a package name instead of the path\u0027s\u0027.","commit_id":"dbf19448b2ff38890f9884a2ab7a2b2c6603d8b6"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2245540b182f164366ea4c501b1dddca5a1f2164","unresolved":true,"context_lines":[{"line_number":35,"context_line":"    path \u003d sys.path"},{"line_number":36,"context_line":"    for name in module_name.split(\u0027.\u0027):"},{"line_number":37,"context_line":"        spec \u003d importlib.util.find_spec(name, path)"},{"line_number":38,"context_line":"        path \u003d [spec.origin, ]"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    return path[0]"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c55ddd30_83a12120","line":38,"updated":"2022-01-18 02:36:50.000000000","message":"I think the new logic looks impatible with the original code.\n\nAssume module_name is \u0027horizon.tables\u0027.\n\nWhen we use imp,\n\n \u003e\u003e\u003e r \u003d imp.find_module(\u0027horizon\u0027, sys.path)\n \u003e\u003e\u003e r\n (None, \u0027/Users/motoki/src/opendev.org/openstack/horizon/horizon\u0027, (\u0027\u0027, \u0027\u0027, 5))\n \u003e\u003e\u003e r[1]\n \u0027/Users/motoki/src/opendev.org/openstack/horizon/horizon\u0027\n \u003e\u003e\u003e imp.find_module(\u0027tables\u0027, [r[1]])\n (None, \u0027/Users/motoki/src/opendev.org/openstack/horizon/horizon/tables\u0027, (\u0027\u0027, \u0027\u0027, 5))\n\nWhen we use importlib,\n\n \u003e\u003e\u003e r \u003d importlib.util.find_spec(\u0027horizon\u0027, sys.path)\n \u003e\u003e\u003e r\n ModuleSpec(name\u003d\u0027horizon\u0027, loader\u003d\u003c_frozen_importlib_external.SourceFileLoader object at 0x1081c6460\u003e, origin\u003d\u0027/Users/motoki/src/opendev.org/openstack/horizon/horizon/__init__.py\u0027, submodule_search_locations\u003d[\u0027/Users/motoki/src/opendev.org/openstack/horizon/horizon\u0027])\n \u003e\u003e\u003e r.origin\n\u0027/Users/motoki/src/opendev.org/openstack/horizon/horizon/__init__.py\u0027\n\nThe point is that there is a case where \u0027spec.origin\u0027 contains not a directory but a file path. In this case, spec.origin cannot be used as \"path\" here.\nAs a result, the new logic cannot find a module path for \u0027horizon.tables\u0027.","commit_id":"dbf19448b2ff38890f9884a2ab7a2b2c6603d8b6"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"532615582aef9eb561dd3a81fc4c97c4ea306841","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    path \u003d sys.path"},{"line_number":36,"context_line":"    for name in module_name.split(\u0027.\u0027):"},{"line_number":37,"context_line":"        spec \u003d importlib.util.find_spec(name, path)"},{"line_number":38,"context_line":"        path \u003d [spec.origin, ]"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    return path[0]"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"6a003431_ddcb6479","line":38,"in_reply_to":"46babc31_71cc3f7f","updated":"2022-01-18 07:10:57.000000000","message":"I think the usage of find_spec you descried is correct. The previous implementation was wrong and doesn\u0027t try checking each element, which I\u0027ve fixed in the latest version.","commit_id":"dbf19448b2ff38890f9884a2ab7a2b2c6603d8b6"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"14fa72b7ff5d23a2cb2ed8e0e1293ecdd601169c","unresolved":true,"context_lines":[{"line_number":35,"context_line":"    path \u003d sys.path"},{"line_number":36,"context_line":"    for name in module_name.split(\u0027.\u0027):"},{"line_number":37,"context_line":"        spec \u003d importlib.util.find_spec(name, path)"},{"line_number":38,"context_line":"        path \u003d [spec.origin, ]"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    return path[0]"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"46babc31_71cc3f7f","line":38,"in_reply_to":"c55ddd30_83a12120","updated":"2022-01-18 04:33:54.000000000","message":"I wonder this function get_module_path() can be replaced like below:\n\n  spec \u003d importlib.util.find_spec(module_name)\n  return spec.origin\n\nget_module_path() is used only in this file (L.106 below).\nPerhaps we can drop get_module_path() function and use importlib directly around L.106.","commit_id":"dbf19448b2ff38890f9884a2ab7a2b2c6603d8b6"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b785ed0fe8a42a5df43c47e97d836bd0b79e32ed","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    path \u003d sys.path"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    elements \u003d module_name.split(\u0027.\u0027)"},{"line_number":37,"context_line":"    for idx in range(len(elements)):"},{"line_number":38,"context_line":"        mod_name \u003d elements[0:idx].join(\u0027.\u0027)"},{"line_number":39,"context_line":"        try:"},{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bbd1bd32_d8886ab3","line":37,"updated":"2022-01-21 05:33:35.000000000","message":"basic question: Why do we need to loop one by one from the parent?\nIs it becuase find_spec(\u0027foo.bar.baz\u0027) raises an ModuleNotFoundError if \u0027foo.bar\u0027 does not exist, right?","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"12b5c3ede85d45a9a8f7bd54852b08ae1e22bb77","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    path \u003d sys.path"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    elements \u003d module_name.split(\u0027.\u0027)"},{"line_number":37,"context_line":"    for idx in range(len(elements)):"},{"line_number":38,"context_line":"        mod_name \u003d elements[0:idx].join(\u0027.\u0027)"},{"line_number":39,"context_line":"        try:"},{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"69d5f675_70dd318c","line":37,"in_reply_to":"85da3967_2a168811","updated":"2022-01-21 05:58:42.000000000","message":"Hmm... this was also wrong and imp.find_module would fail with ImortError if the module_name is not actually a module_name. I\u0027ll revert this back...","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"80eb3e21e23443fcfd4ab6c1a0e6c74916f0057a","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    path \u003d sys.path"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    elements \u003d module_name.split(\u0027.\u0027)"},{"line_number":37,"context_line":"    for idx in range(len(elements)):"},{"line_number":38,"context_line":"        mod_name \u003d elements[0:idx].join(\u0027.\u0027)"},{"line_number":39,"context_line":"        try:"},{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"85da3967_2a168811","line":37,"in_reply_to":"bbd1bd32_d8886ab3","updated":"2022-01-21 05:44:52.000000000","message":"My understanding about this implementation is that it deals with the case where foo.baa is the module name and baz is the name of an object defined in that module(like os.path.exists ).\nIn this case we can\u0027t use the whole path but need to us the actual module name(os.path) to which the instance belongs.","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b785ed0fe8a42a5df43c47e97d836bd0b79e32ed","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    elements \u003d module_name.split(\u0027.\u0027)"},{"line_number":37,"context_line":"    for idx in range(len(elements)):"},{"line_number":38,"context_line":"        mod_name \u003d elements[0:idx].join(\u0027.\u0027)"},{"line_number":39,"context_line":"        try:"},{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"},{"line_number":41,"context_line":"            if spec:"}],"source_content_type":"text/x-python","patch_set":6,"id":"018c39ba_b6e3de57","line":38,"updated":"2022-01-21 05:33:35.000000000","message":"elements[0:0] returns []. I wonder idx needs to start from 1.","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"80eb3e21e23443fcfd4ab6c1a0e6c74916f0057a","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    elements \u003d module_name.split(\u0027.\u0027)"},{"line_number":37,"context_line":"    for idx in range(len(elements)):"},{"line_number":38,"context_line":"        mod_name \u003d elements[0:idx].join(\u0027.\u0027)"},{"line_number":39,"context_line":"        try:"},{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"},{"line_number":41,"context_line":"            if spec:"}],"source_content_type":"text/x-python","patch_set":6,"id":"e6824dd0_f0b5fef8","line":38,"in_reply_to":"018c39ba_b6e3de57","updated":"2022-01-21 05:44:52.000000000","message":"exactly... I\u0027ll fix it.","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b785ed0fe8a42a5df43c47e97d836bd0b79e32ed","unresolved":true,"context_lines":[{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"},{"line_number":41,"context_line":"            if spec:"},{"line_number":42,"context_line":"                path \u003d [spec.origin, ]"},{"line_number":43,"context_line":"        except AttributeError:"},{"line_number":44,"context_line":"            pass"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"3c148eb1_58430eee","line":43,"range":{"start_line":43,"start_character":8,"end_line":43,"end_character":30},"updated":"2022-01-21 05:33:35.000000000","message":"question: when is AttributeError raised?\nDoesn\u0027t \u0027origin\u0027 attribute always exist when spec is not None?","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"3627447fe93bdb3038eaf61634fc9d4aa2557654","unresolved":true,"context_lines":[{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"},{"line_number":41,"context_line":"            if spec:"},{"line_number":42,"context_line":"                path \u003d [spec.origin, ]"},{"line_number":43,"context_line":"        except AttributeError:"},{"line_number":44,"context_line":"            pass"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"e92cd61f_c412f23f","line":43,"range":{"start_line":43,"start_character":8,"end_line":43,"end_character":30},"in_reply_to":"27ce47f7_345bc9ca","updated":"2022-01-21 05:49:09.000000000","message":"Hmm... it might be wrong that I\u0027m testing it with os.path. I\u0027ll recheck this.","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"80eb3e21e23443fcfd4ab6c1a0e6c74916f0057a","unresolved":true,"context_lines":[{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"},{"line_number":41,"context_line":"            if spec:"},{"line_number":42,"context_line":"                path \u003d [spec.origin, ]"},{"line_number":43,"context_line":"        except AttributeError:"},{"line_number":44,"context_line":"            pass"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"27ce47f7_345bc9ca","line":43,"range":{"start_line":43,"start_character":8,"end_line":43,"end_character":30},"in_reply_to":"3c148eb1_58430eee","updated":"2022-01-21 05:44:52.000000000","message":"I misunderstood the traceback but it seems ModuleNotFoundError is raised when importing an object instead of a module.\n\n```\n\u003e\u003e\u003e importlib.util.find_spec(\u0027os.path.exists\u0027)\nTraceback (most recent call last):\n  File \"/usr/lib64/python3.8/importlib/util.py\", line 96, in find_spec\n    parent_path \u003d parent.__path__\nAttributeError: module \u0027posixpath\u0027 has no attribute \u0027__path__\u0027\n\nThe above exception was the direct cause of the following exception:\n\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"/usr/lib64/python3.8/importlib/util.py\", line 98, in find_spec\n    raise ModuleNotFoundError(\nModuleNotFoundError: __path__ attribute not found on \u0027os.path\u0027 while trying to find \u0027os.path.exists\u0027\n```","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2b096d46c105c5fbda39d050127eafb213f253e2","unresolved":true,"context_lines":[{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"},{"line_number":41,"context_line":"            if spec:"},{"line_number":42,"context_line":"                path \u003d [spec.origin, ]"},{"line_number":43,"context_line":"        except AttributeError:"},{"line_number":44,"context_line":"            pass"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"0f01cd5f_b8fe2341","line":43,"range":{"start_line":43,"start_character":8,"end_line":43,"end_character":30},"in_reply_to":"4e238c5d_99f6f7a5","updated":"2022-01-21 07:02:14.000000000","message":"This function get_module_path() expects a module name as an argument, so I don\u0027t think we need to take account into the case with an attribute like openstack_dashboard.settings.DEBUG.","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"12b5c3ede85d45a9a8f7bd54852b08ae1e22bb77","unresolved":true,"context_lines":[{"line_number":40,"context_line":"            spec \u003d importlib.util.find_spec(mod_name)"},{"line_number":41,"context_line":"            if spec:"},{"line_number":42,"context_line":"                path \u003d [spec.origin, ]"},{"line_number":43,"context_line":"        except AttributeError:"},{"line_number":44,"context_line":"            pass"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return path[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"4e238c5d_99f6f7a5","line":43,"range":{"start_line":43,"start_character":8,"end_line":43,"end_character":30},"in_reply_to":"e92cd61f_c412f23f","updated":"2022-01-21 05:58:42.000000000","message":"I reproduced the same with openstack_dashboards.settings.DEBUG for example but this is not a valid use case according to my observation to the first point. So I revert this back.","commit_id":"239111fc38ea15fc47b23f4797c822fce725c6cb"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2b096d46c105c5fbda39d050127eafb213f253e2","unresolved":true,"context_lines":[{"line_number":32,"context_line":"    Avoids conflicts with package dependencies."},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":"    spec \u003d importlib.util.find_spec(module_name)"},{"line_number":35,"context_line":"    return spec.origin if spec else sys.path"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class DirContext(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"498fe77e_1eb1ef5c","line":35,"updated":"2022-01-21 07:02:14.000000000","message":"This code raises ModuleNotFoundError if non-existing module is specified as a parent module (e.g., \"foo.bar\" does not exist for \"foo.bar.baz\"). Howeber, I am not sure we need to cover such cases. The return value of this function is used to calculate the path for \"local\" dir for the dashboard settings. In addition, DJANGO_SETTINGS_MODULE should be configured properly by deployers/packagers. Thus, I am fine with this logic.\n\n(Honestly, I don\u0027t understand why sys.path needs to be considered, but it is a separate thing.)","commit_id":"561b25ccc08e9605e6fafd5a1176378237f95285"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"5d551b103e6a7bee9b57114d6fbb252e2ff273d4","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Avoids conflicts with package dependencies."},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":"    spec \u003d importlib.util.find_spec(module_name)"},{"line_number":35,"context_line":"    return spec.origin if spec else sys.path"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class DirContext(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"508a2e87_baff96c1","line":35,"in_reply_to":"498fe77e_1eb1ef5c","updated":"2022-01-21 07:09:35.000000000","message":"Hmm... sys.path is used for the first iteration of find_module but it should not be used after that. If the module can\u0027t be loaded then find_module raises an Exception thus it does never return sys.path... so we should avoud returning sys.path actually.","commit_id":"561b25ccc08e9605e6fafd5a1176378237f95285"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"658048f514220819322a7f2783e0d05f545febf4","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Avoids conflicts with package dependencies."},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":"    spec \u003d importlib.util.find_spec(module_name)"},{"line_number":35,"context_line":"    return spec.origin if spec else sys.path"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class DirContext(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"35ddcc95_36b534fa","line":35,"in_reply_to":"508a2e87_baff96c1","updated":"2022-01-21 07:59:25.000000000","message":"Yeah, thanks for the update.","commit_id":"561b25ccc08e9605e6fafd5a1176378237f95285"}]}
