[07:04:59] (03CR) 10Hashar: [C: 04-1] commands: Replace subprocess.check_call with subprocess.run (031 comment) [integration/quibble] - 10https://gerrit.wikimedia.org/r/895854 (https://phabricator.wikimedia.org/T331061) (owner: 10Kosta Harlan) [07:17:58] (03CR) 10Kosta Harlan: commands: Replace subprocess.check_call with subprocess.run (031 comment) [integration/quibble] - 10https://gerrit.wikimedia.org/r/895854 (https://phabricator.wikimedia.org/T331061) (owner: 10Kosta Harlan) [08:11:24] (03Restored) 10Kosta Harlan: DNM: CI test [integration/quibble] - 10https://gerrit.wikimedia.org/r/896018 (owner: 10Kosta Harlan) [08:12:42] (03CR) 10Kosta Harlan: "recheck" [integration/quibble] - 10https://gerrit.wikimedia.org/r/896018 (owner: 10Kosta Harlan) [08:16:56] (03CR) 10Kosta Harlan: "recheck" [integration/quibble] - 10https://gerrit.wikimedia.org/r/895854 (https://phabricator.wikimedia.org/T331061) (owner: 10Kosta Harlan) [08:20:04] hashar: btw I thought subprocess.run might have broken real time output per your code review comment, I am watching https://integration.wikimedia.org/ci/job/integration-quibble-fullrun-defaults/19/console to check [08:20:26] yeah sorry the whole change confuses me [08:20:36] well not your change really, but subprocess does :D [08:20:39] it does not seem to break realtime output [08:21:00] yes, me as well :\ [08:21:17] I can only say that subprocess.run works for what I want :D [08:26:07] :D [08:29:37] hm [08:29:42] I'm not sure about stdout=subprocess.PIPE [08:30:13] I was just checking that the MW_QUIBBLE_CI environment variable propagates if I set it in cmd.py#_setup_environment [08:30:14] and it does [08:30:32] but I was checking by doing `var_dump( $_SERVER );` in MediaWikiUnitTestCase and there was no output [08:30:47] if I remove the `stdout=subprocess.PIPE` line from `_subprocess_run()` then I do see the output. [08:30:54] my guess is with stdout=subprocess.PIPE the output is returned via CompletedProcess [08:31:02] ah [08:31:23] but why does the output shows in the Jenkins console so? :) [08:31:30] that is the part I don't get [08:32:11] (03PS4) 10Kosta Harlan: env: Export MW_QUIBBLE_CI [integration/quibble] - 10https://gerrit.wikimedia.org/r/896084 (https://phabricator.wikimedia.org/T331621) [08:32:13] (03PS7) 10Kosta Harlan: release: Quibble 1.5.3 [integration/quibble] - 10https://gerrit.wikimedia.org/r/896061 (https://phabricator.wikimedia.org/T331061) [08:32:15] (03PS7) 10Kosta Harlan: release: Start 1.5.4 cycle [integration/quibble] - 10https://gerrit.wikimedia.org/r/896062 [08:32:22] 🤷 [08:34:15] https://pypi.org/project/subprocess-tee/ :D [08:35:01] heh. I think I came across this when doing my googling about how to implement this [08:35:08] there's lots of dated stackoverflow snippets as well [08:36:01] there is something I wrote with a stream to relay the output to logging [08:36:07] which uses a thread for the reading [08:36:17] requires python 3.8 [08:36:31] ^ subprocess-tee [08:36:49] yeah and I don't think we should use it, but that can give insights [08:36:55] I will dig into it this afternoon , I wanna understand why the output is actually shown [08:44:58] hashar: https://stackoverflow.com/questions/4417546/constantly-print-subprocess-output-while-process-is-running might be helpful (or just more confusion) [08:45:15] (03CR) 10Kosta Harlan: env: Export MW_QUIBBLE_CI (032 comments) [integration/quibble] - 10https://gerrit.wikimedia.org/r/896084 (https://phabricator.wikimedia.org/T331621) (owner: 10Kosta Harlan) [08:47:59] (03CR) 10Hashar: [C: 03+2] "Looks simpler this way, thank you!" [integration/quibble] - 10https://gerrit.wikimedia.org/r/896084 (https://phabricator.wikimedia.org/T331621) (owner: 10Kosta Harlan) [08:48:16] I have an appointment this morning [08:48:23] i will mess with subprocess after lunch [08:48:40] and I am pretty sure the explanation will be completely obvious once found [09:29:46] 🤞 [09:57:28] if you want, we can pair on it this afternoon, I sent you an invite [13:35:57] kostajh: back sorry I had an appointment and an extended lunch [13:36:00] for a change :] [13:40:00] hashar: sounds nice! (the lunch part anyway) [13:40:13] i'm around if you want to pair on it, otherwise, I can review whatever you come up with or we can try for next week [13:40:16] sure [13:40:37] but I swear I don't' understand how the commands outputs end up showing up :( [13:43:56] hehe [13:44:43] reading https://stackoverflow.com/questions/40697583/whats-the-difference-between-pythons-subprocess-call-and-subprocess-run [13:45:04] and now on to https://docs.python.org/3/library/subprocess.html#older-high-level-api [13:45:33] the alternative is to keep the check_call [13:45:43] and do the unicode decoding when emittiing the report [13:45:50] but then you don't have the output of the command :D [13:46:20] with your patch apparently you get the output of the command AND it gets written to stdout which to me is an entire mystery [13:46:24] it should not work [13:47:06] yeah check_call does not give us the output [13:48:28] why do you say it should not work? [13:48:34] https://docs.python.org/3/library/subprocess.html#subprocess.run [13:55:59] cause subprocess.run(stdout=subprocess.PIPE) captures the output in a pipe? [13:56:05] and thus nothing is displayed to stdout? [13:56:20] or I am completely mislead? [14:04:16] ohhhh [14:04:21] the output is send to stderr [14:04:40] if I invoke quibble with `> /dev/null` I still show output :) [14:06:04] I am testing with `--run phpunit-unit` [14:08:16] so that would not capture stderr (which might have the actual failure) [14:08:51] we can capture both using `stdout=subprocess.PIPE` `stderr,subprocess.STDOUT` [14:09:12] which would get both of them multiplexed and attached to `CalledProcessError.output` if it is raised [14:09:26] and would hide the progress entirely from the console output [14:09:30] aka no streaming [14:09:35] mystery solve [14:11:23] so we would have to with the lower level `subprocess.Popen(stdout=subprocess.PIPE, stderr=subprocess.STDOUT)` [14:11:43] and continuously update its output iterating on it [14:14:54] with Popen(["ifconfig"], stdout=PIPE, stderr=STDOUT) as proc: [14:14:55] print(proc.stdout.read()) [14:15:21] we can still pair if you want :] [14:15:27] but I am happy to have resolve tha tmystery [14:45:46] (03CR) 10Hashar: [C: 04-1] commands: Replace subprocess.check_call with subprocess.run (032 comments) [integration/quibble] - 10https://gerrit.wikimedia.org/r/895854 (https://phabricator.wikimedia.org/T331061) (owner: 10Kosta Harlan) [15:12:00] (03CR) 10Hashar: [C: 04-1] commands: Replace subprocess.check_call with subprocess.run (031 comment) [integration/quibble] - 10https://gerrit.wikimedia.org/r/895854 (https://phabricator.wikimedia.org/T331061) (owner: 10Kosta Harlan) [15:13:29] hashar: do you want to amend my patch? [15:13:45] possibly [15:13:57] I am afk now [15:13:59] I gotta figure out the magic of handling the raw output from the command [15:14:30] Otherwise I can try to implement your suggestions next week. But anyway feel free to edit the internals of my patch [15:14:46] sure. I might cookie lick tonight or over the week-end :] [15:14:52] I guess we can rename _subprocess_run to a more generic run() or something [15:15:55] possibly [15:17:23] I don't quite know about binary vs text modes [16:02:23] (03CR) 10Hashar: [C: 04-1] "I think I found a way to print the data AND capture it even if it has invalid unicode by doing bytes processing. Then the stream of bytes" [integration/quibble] - 10https://gerrit.wikimedia.org/r/895854 (https://phabricator.wikimedia.org/T331061) (owner: 10Kosta Harlan) [16:02:36] kostajh: https://gerrit.wikimedia.org/r/c/integration/quibble/+/896377 :) [16:02:44] it is dirty but seems like it is doing what I want [16:03:16] which might work as long as the command output is not too long :] [16:03:21] I am off for the week-end [16:44:56] cool,I will check it out