)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"c499d38928b9b5e219450210f1af0b0e2062f738","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"089955eb_c1a6313c","updated":"2022-02-25 15:26:51.000000000","message":"recheck","commit_id":"1072b13297cadc2700f64bf35085df1626bab6f2"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"7fe579e3f8501dfe165590f0ba530ab088253d51","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"79652ce9_ea2eabe7","updated":"2022-03-01 07:42:13.000000000","message":"first pass looks good can\u0027t spot something terrible ;)","commit_id":"ce3bb5099a7c63eebc8b9998d59c59fa037e6338"},{"author":{"_account_id":6889,"name":"Fabien Boucher","email":"fboucher@redhat.com","username":"fabien-boucher"},"change_message_id":"86a074dd9406b75a31c787ed5a5c86081203953d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"974d0394_bdeb1d26","updated":"2022-03-01 16:33:48.000000000","message":"I would have separated the downloader tool in a dedicated tool for clarity. Also IMO you should add retry mechanics when you do http calls. See tenacity lib perhaps. ","commit_id":"44deb28198f1e904e1eb7849b303a451b251e570"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"303fc61c4e879326a25b2135d7e9b1d059d5213b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"47f6142c_d01b140d","updated":"2022-03-03 08:41:48.000000000","message":"recheck","commit_id":"44deb28198f1e904e1eb7849b303a451b251e570"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"367b9d49b34e427a673e6340e901b07d08511673","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"a86bcbfe_fd8155d9","updated":"2022-03-03 07:29:11.000000000","message":"recheck","commit_id":"44deb28198f1e904e1eb7849b303a451b251e570"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"3ca9bde866bd2cb4823a5014fc7fcf3082e06d97","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"01b347c9_f0bdbed8","updated":"2022-03-02 07:59:12.000000000","message":"stared some more and still cant see the nits ;)","commit_id":"44deb28198f1e904e1eb7849b303a451b251e570"},{"author":{"_account_id":6889,"name":"Fabien Boucher","email":"fboucher@redhat.com","username":"fabien-boucher"},"change_message_id":"3b3e5f0fefdc0c87c61b8c592b7d681d2e08a7fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"d34b021a_35788e07","in_reply_to":"7e3c2894_ba3a9194","updated":"2022-03-02 09:23:12.000000000","message":"It won\u0027t. With tenacity you should be able to inspect the request result and retry only in case of specific issue (so you can slip a 404). Here what happen if a get request timeout.","commit_id":"44deb28198f1e904e1eb7849b303a451b251e570"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"9743b9f448188fb47cfd4676c8ed007d3b9d2650","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"7e3c2894_ba3a9194","in_reply_to":"974d0394_bdeb1d26","updated":"2022-03-02 07:23:50.000000000","message":"With tenacity, it will try to download files that it does not exists, so what is the sense to download file that it might not be present there?\nNormally we can assume, that on the logserver it should be job-results.txt file, but depends of the provider it can have extension .gz or not, so how to recognize that one provider is doing and one not?\nMaybe in the future zuul post job can push the job results and other important logs directly to logscraper host and then they can be pushed by logsender tool.\nSo far, we should do it as it is.","commit_id":"44deb28198f1e904e1eb7849b303a451b251e570"}],"ansible/roles/check-services/tasks/main.yml":[{"author":{"_account_id":9397,"name":"Allison Randal","email":"allison@lohutok.net","username":"wendar"},"change_message_id":"340e5a788a85a4e885d4b9f9c2098e4a575af249","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        chdir: \"{{ zuul.projects[\u0027opendev.org/openstack/ci-log-processing\u0027].src_dir }}\""},{"line_number":34,"context_line":"      when: zuul is defined"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    - name: Build logscraper container image - non Zuul"},{"line_number":37,"context_line":"      shell: \u003e"},{"line_number":38,"context_line":"        podman build -t quay.io/loggearman:dev -f loggearman/Dockerfile"},{"line_number":39,"context_line":"      args:"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"c0ae0974_5f8f2cc4","line":36,"range":{"start_line":36,"start_character":18,"end_line":36,"end_character":28},"updated":"2022-02-28 17:04:33.000000000","message":"Should this be \"loggearman\" instead of \"logscraper\"?","commit_id":"fafa8b4ba70242479e1b2afb3dc6ad0111dc8be9"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"23326e40feec32430b5e7be0f06da3ee5ba3b59b","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        chdir: \"{{ zuul.projects[\u0027opendev.org/openstack/ci-log-processing\u0027].src_dir }}\""},{"line_number":34,"context_line":"      when: zuul is defined"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    - name: Build logscraper container image - non Zuul"},{"line_number":37,"context_line":"      shell: \u003e"},{"line_number":38,"context_line":"        podman build -t quay.io/loggearman:dev -f loggearman/Dockerfile"},{"line_number":39,"context_line":"      args:"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"53b2d969_ec502c5e","line":36,"range":{"start_line":36,"start_character":18,"end_line":36,"end_character":28},"in_reply_to":"3bda724d_d2a7f747","updated":"2022-03-01 07:15:04.000000000","message":"Done","commit_id":"fafa8b4ba70242479e1b2afb3dc6ad0111dc8be9"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"98874956dbd6c7d0cedb6e514136a3babd256350","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        chdir: \"{{ zuul.projects[\u0027opendev.org/openstack/ci-log-processing\u0027].src_dir }}\""},{"line_number":34,"context_line":"      when: zuul is defined"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    - name: Build logscraper container image - non Zuul"},{"line_number":37,"context_line":"      shell: \u003e"},{"line_number":38,"context_line":"        podman build -t quay.io/loggearman:dev -f loggearman/Dockerfile"},{"line_number":39,"context_line":"      args:"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"3bda724d_d2a7f747","line":36,"range":{"start_line":36,"start_character":18,"end_line":36,"end_character":28},"in_reply_to":"c0ae0974_5f8f2cc4","updated":"2022-02-28 17:23:58.000000000","message":"good point, it should :)","commit_id":"fafa8b4ba70242479e1b2afb3dc6ad0111dc8be9"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"7fe579e3f8501dfe165590f0ba530ab088253d51","unresolved":true,"context_lines":[{"line_number":18,"context_line":"      shell: \u003e"},{"line_number":19,"context_line":"        podman build -t quay.io/logscraper:dev -f Dockerfile"},{"line_number":20,"context_line":"      args:"},{"line_number":21,"context_line":"        chdir: \"{{ playbook_dir }}\""},{"line_number":22,"context_line":"      when: zuul is not defined"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    - name: Get logscraper image id"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"b346cee0_dc3cf784","line":21,"updated":"2022-03-01 07:42:13.000000000","message":"having two tasks like this is fine.. mostly out of curiosity i wonder if you could have used somehting like:\n\n           chdir: \"{{ playbook_dir }} if zuul is not defined else {{ zuul.projects[\u0027opendev.org/openstack/ci-log-processing\u0027].src_dir }}\"","commit_id":"ce3bb5099a7c63eebc8b9998d59c59fa037e6338"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"3ca9bde866bd2cb4823a5014fc7fcf3082e06d97","unresolved":false,"context_lines":[{"line_number":18,"context_line":"      shell: \u003e"},{"line_number":19,"context_line":"        podman build -t quay.io/logscraper:dev -f Dockerfile"},{"line_number":20,"context_line":"      args:"},{"line_number":21,"context_line":"        chdir: \"{{ playbook_dir }}\""},{"line_number":22,"context_line":"      when: zuul is not defined"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    - name: Get logscraper image id"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"14f75270_f6faba59","line":21,"in_reply_to":"af630a54_fd6d03d7","updated":"2022-03-02 07:59:12.000000000","message":"Done","commit_id":"ce3bb5099a7c63eebc8b9998d59c59fa037e6338"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"0c8b493333cb93325754388c4911201c2ed58abe","unresolved":true,"context_lines":[{"line_number":18,"context_line":"      shell: \u003e"},{"line_number":19,"context_line":"        podman build -t quay.io/logscraper:dev -f Dockerfile"},{"line_number":20,"context_line":"      args:"},{"line_number":21,"context_line":"        chdir: \"{{ playbook_dir }}\""},{"line_number":22,"context_line":"      when: zuul is not defined"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    - name: Get logscraper image id"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"af630a54_fd6d03d7","line":21,"in_reply_to":"b346cee0_dc3cf784","updated":"2022-03-01 11:19:54.000000000","message":"I don\u0027t like to put condition into the task (same to mix jinja2 template into the task)","commit_id":"ce3bb5099a7c63eebc8b9998d59c59fa037e6338"}],"logscraper/logscraper.py":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"7fe579e3f8501dfe165590f0ba530ab088253d51","unresolved":true,"context_lines":[{"line_number":485,"context_line":"        try:"},{"line_number":486,"context_line":"            if not os.path.exists(directory):"},{"line_number":487,"context_line":"                os.makedirs(directory)"},{"line_number":488,"context_line":"        except PermissionError:"},{"line_number":489,"context_line":"            logging.critical(\"Can not create directory %s\" % directory)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        check_specified_files(build, args.insecure, directory)"}],"source_content_type":"text/x-python","patch_set":9,"id":"11df024e_c4f1ad99","line":488,"range":{"start_line":488,"start_character":15,"end_line":488,"end_character":30},"updated":"2022-03-01 07:42:13.000000000","message":"just wonder if that is too specific there could be other error?","commit_id":"ce3bb5099a7c63eebc8b9998d59c59fa037e6338"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"3ca9bde866bd2cb4823a5014fc7fcf3082e06d97","unresolved":false,"context_lines":[{"line_number":485,"context_line":"        try:"},{"line_number":486,"context_line":"            if not os.path.exists(directory):"},{"line_number":487,"context_line":"                os.makedirs(directory)"},{"line_number":488,"context_line":"        except PermissionError:"},{"line_number":489,"context_line":"            logging.critical(\"Can not create directory %s\" % directory)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        check_specified_files(build, args.insecure, directory)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fc93301_1f122f06","line":488,"range":{"start_line":488,"start_character":15,"end_line":488,"end_character":30},"in_reply_to":"11df024e_c4f1ad99","updated":"2022-03-02 07:59:12.000000000","message":"Done","commit_id":"ce3bb5099a7c63eebc8b9998d59c59fa037e6338"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"3ca9bde866bd2cb4823a5014fc7fcf3082e06d97","unresolved":false,"context_lines":[{"line_number":487,"context_line":"                os.makedirs(directory)"},{"line_number":488,"context_line":"        except PermissionError:"},{"line_number":489,"context_line":"            logging.critical(\"Can not create directory %s\" % directory)"},{"line_number":490,"context_line":"        except Exception as e:"},{"line_number":491,"context_line":"            logging.critical(\"Exception occured %s on creating dir %s\" % ("},{"line_number":492,"context_line":"                e, directory))"},{"line_number":493,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"e6eb2a45_4c050ff0","line":490,"range":{"start_line":490,"start_character":15,"end_line":490,"end_character":24},"updated":"2022-03-02 07:59:12.000000000","message":"ack thanks","commit_id":"44deb28198f1e904e1eb7849b303a451b251e570"}]}