| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 856317: Add option to include returned data in MQTT reporter https://review.opendev.org/c/zuul/zuul/+/856317 | 01:05 | |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/zuul] 855309: zuul-stream: Add variable to disable writing streaming files https://review.opendev.org/c/zuul/zuul/+/855309 | 01:20 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] 857421: Trace merge requests and merger operations https://review.opendev.org/c/zuul/zuul/+/857421 | 04:59 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] 857421: Trace merge requests and merger operations https://review.opendev.org/c/zuul/zuul/+/857421 | 05:02 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] 857421: Trace merge requests and merger operations https://review.opendev.org/c/zuul/zuul/+/857421 | 08:45 | |
| @clarkb:matrix.org | Re ansible slowness this whole document has been useful for understanding what is going on, but this section in particular seems important. https://docs.ansible.com/ansible/latest/dev_guide/developing_program_flow_modules.html#ansiballz-framework I half wonder if we are generating some very large zipfile due to transitive imports of module utils. | 15:22 |
|---|---|---|
| @clarkb:matrix.org | I'm not sure any of that is actionable, but gives good background for anyone trying to better understand how this works under the hood | 15:22 |
| @clarkb:matrix.org | and I half wonder if ansible 5 has regressed in performance around this stuff. Though it is entirely possible I've just managed to notice recently and its been an issue all along. I know we've looked into this in the past for similar reasons | 15:25 |
| -@gerrit:opendev.org- Alfredo Moralejo proposed: [zuul/zuul-jobs] 857730: Use AFS mirrors for extras-common in CS9 https://review.opendev.org/c/zuul/zuul-jobs/+/857730 | 15:36 | |
| @clarkb:matrix.org | Doing some local testing to do some expectation setting running 2 command tasks to capture `date +%s.%N` for time measurement and 5 file copies in a loop of a 91 byte file takes just under 4 seconds. Which is inline with what corvus was measuring at about half a second per task | 15:51 |
| @clarkb:matrix.org | the difference between enabling and disabling pipelining for that is minimal. Makes me wonder if it is enabeld by default unless you use become or something. Or maybe disk io is negligible with the size of the files being used here | 15:52 |
| @clarkb:matrix.org | Using the ansible task debugger (did you know ansible has a task debugger?) I was able to essentially put a breakpoint in place and grab the ansiballz base64 encoded zip files containing the python code for executing the copy module | 16:10 |
| @clarkb:matrix.org | the base64 file is 129kb, decoded it is 97k zip file, and inflating the zip file results in a directory structure about 440kb | 16:11 |
| @clarkb:matrix.org | nothing crazy | 16:11 |
| @clarkb:matrix.org | An interesting thign I see in my debug output is a lot of sftp of files which implies to me that pipelining isn't enabled even though I'm running with ANSIBLE_SSH_PIPELINING=1. Elsewhere on the internet people claim copy with pipelining is slower than without pipeling though. https://stackoverflow.com/questions/43438519/check-if-ansible-pipelining-is-enabled-working | 16:17 |
| @clarkb:matrix.org | I'm going to see if I can't reproduce those results by actually enabling pipelining | 16:17 |
| @clarkb:matrix.org | Using ANSIBLE_PIPELINING=1 updates the debug output to say pipelining is explicitly enabled. Maybe it needs to be enabled globally and per connection separately? | 16:20 |
| @clarkb:matrix.org | I still see it using sftp tocopy the file I want to write which makes sense given that is probably much quicker than streaming it through stdin to the python process. I wonder if that is the fix for the slowness others have reported half a decade ago | 16:21 |
| @clarkb:matrix.org | And now that pipelining is actually enabled the runtime goes to just under 3 seconds instead of just under 4 seconds for the same set of tasks. An improvement if not a large one | 16:22 |
| @clarkb:matrix.org | corvus: in the zuul test suite when we set self.executor_server.verbose = True do you know where that ansible logging is written to or how I can check its contents from within a test? In local by hand testing of ansible setting ssh_connection.pipelining does not actually enable pipelining and this is what zuul does. | 16:42 |
| @clarkb:matrix.org | corvus: setting connection.pipelining to true or defaults.pipelining does enable it | 16:42 |
| @jim:acmegating.com | Clark: i think it would be in the executor debug log; you can use a contextmanager to capture that output | 16:59 |
| @jim:acmegating.com | Clark: (mostly those are enabled in tests so that the debug output is visible to humans if the test fails) | 16:59 |
| @jim:acmegating.com | Clark: oh, actually, look at test_job_output_failure_log as maybe the best way to deal with log output in tests | 17:00 |
| @jim:acmegating.com | i think i like that better than the context manager | 17:00 |
| @clarkb:matrix.org | thanks | 17:01 |
| @clarkb:matrix.org | curiously if I run `ansible-config init --disabled -t all > ansible.cfg` the resulting output does show ssh_connection.pipelining as defaulting to ANSIBLE_PIPELINING | 17:01 |
| @clarkb:matrix.org | it almost appears that we aren't overriding that via the ansible.cfg file for some reason when I run it locally | 17:02 |
| @jim:acmegating.com | Clark: (with self.assertLogs was the context manager i was thinking of, but as i said, i think the logger attachment may be better since it still outputs as normal) | 17:02 |
| @clarkb:matrix.org | But ya I'd like to have the zuul test suite check it against how zuul is configured | 17:02 |
| @clarkb:matrix.org | If I run `ANSIBLE_CONFIG=/home/clark/.ansible.cfg ~/tmp/ansibleenv/bin/ansible-config dump --only-changed -t all` ansible shows my config is different (separately it is weird to me that i have to explicitly set ANSIBLE_CONFIG to the default lookup path I don't understand that either). But when I run it I don't get pipelining. If I set connection.pipelining or defaults.pipelining in the same file then it works | 17:08 |
| @clarkb:matrix.org | https://github.com/ansible/ansible/issues/76572 I'm reading this now | 17:10 |
| @clarkb:matrix.org | I'm 99% sure this is our issue | 17:11 |
| @clarkb:matrix.org | Unfortunately I have no idea how to parse https://github.com/ansible/ansible/issues/76572#issuecomment-1157743700 | 17:12 |
| @clarkb:matrix.org | If no backport was done does that mean ansible 5/2.12 is just broken forever? | 17:13 |
| @clarkb:matrix.org | and the issue is locked so I can't even request clarification | 17:14 |
| @clarkb:matrix.org | I'm going to make sure my local ansible install is up to date I guess | 17:14 |
| @clarkb:matrix.org | what I have installed is from august 15 (2.12.8) which is newer than the last update on that issue so I think they really didn't fix ansible 5 | 17:15 |
| @clarkb:matrix.org | but 2.12.9 exists so I will update to that and retes | 17:15 |
| @clarkb:matrix.org | Nevermind. Ansible 5.10.0 is what wants ansible-core 2.12.8. There doesn't appear to be an ansible 5 release that does 2.12.9 so I think this was just never fixed in ansible 5. Joy | 17:17 |
| @jim:acmegating.com | Clark: good thing we're about to move to 6? :) | 17:18 |
| @clarkb:matrix.org | I guess? I'm frustrated that upstream thought this was small enough to just ignore | 17:18 |
| @clarkb:matrix.org | and their versioning is completely indecipherable so I can't make sense of what I need to get the update | 17:19 |
| @clarkb:matrix.org | ansible-base is apparently different than ansible-core and ansible? | 17:19 |
| @clarkb:matrix.org | I'm going to update to ansible 6 and retest as ansible base hasn't updated since january | 17:19 |
| @jim:acmegating.com | i think ansible-base was a one-time/transition thing | 17:20 |
| @clarkb:matrix.org | gotcha. fwiw the code appears to be in what I think is ansible-core | 17:21 |
| @jim:acmegating.com | yep | 17:21 |
| @jim:acmegating.com | https://docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#ansible-core-changelogs | 17:21 |
| @clarkb:matrix.org | ansible 6 does fix it | 17:21 |
| @clarkb:matrix.org | Is there any reason we can't fix this for 5 too in zuul by adding it to the defaults section? I know we support other connection types and they may not want to use pipeling? | 17:22 |
| @clarkb:matrix.org | I can write that patch and try to add a regression testing by checking ansible debug output for the pipelining string, just not sure how safe that change is | 17:23 |
| @jim:acmegating.com | Clark: is this so bad that we need to? i mean, we're literally adding ansible 6 as soon as opendev finishes restarting... | 17:23 |
| @clarkb:matrix.org | corvus: I don't think we should move openstack until they release though | 17:23 |
| @clarkb:matrix.org | so we'll live with this for about another month or so likely | 17:24 |
| @jim:acmegating.com | basically, if it works but is a bit slower in some cases, i'm not convinced we should open the can of worms by moving it to defaults | 17:24 |
| @jim:acmegating.com | if the long-term fix is going to merge in a couple days | 17:24 |
| @clarkb:matrix.org | ya I think that is fair | 17:24 |
| @clarkb:matrix.org | since it will affect everything | 17:24 |
| @clarkb:matrix.org | kubernetes, winrm etc | 17:24 |
| @clarkb:matrix.org | I would happily leave a comment on that issue asking ansible to backport the fix | 17:25 |
| @clarkb:matrix.org | But I'm not allowed ot | 17:25 |
| @clarkb:matrix.org | corvus: this is good for the zuul messaging around updating to 6 though. "Update to 6 to get pipelining back which should make your jobs faster" | 17:25 |
| @clarkb:matrix.org | I guess what they are saying is ansible 5 is EOL. I thought they did more overlap than that | 17:32 |
| @clarkb:matrix.org | It is odd that they never did an ansible 5 release with 2.12.9 though | 17:32 |
| @clarkb:matrix.org | or maybe they did and pip is being too smart for me preexisting isntall | 17:33 |
| @jim:acmegating.com | afaik, there's no overlap between ansible releases; i'm not even sure whether there will be overlap in ansible-core in the future or not... | 17:36 |
| @clarkb:matrix.org | they did release 5.10.0 after 6.0.0 but only a week later | 17:36 |
| @clarkb:matrix.org | which is pretty close to no overlap | 17:37 |
| @jim:acmegating.com | it's going to make for fun zuul releases | 17:37 |
| @jim:acmegating.com | zuul-maint: the opendev restart is finished (thanks Clark !) and most of the servers are running master now (some executors are a few commits behind, but those commits aren't relevant to executors, or even to opendev). so how does this look for a zuul release tomorrow? commit a4fdfba1e3f07d554d6625413154c804d3c64d6f (HEAD -> master, tag: 6.4.0, origin/master, gerrit/master) | 20:09 |
| @fungicide:matrix.org | a4fdfba as 6.4.0 lgtm! | 20:34 |
| @clarkb:matrix.org | Same here | 20:39 |
| @clarkb:matrix.org | corvus: now that we've got a commit selected any concern with me approving changes? I was going to look at ianw's web cleanup | 21:24 |
| @iwienand:matrix.org | Clark: am i parsing things right that you've found our recent regression in speed on things like system-config are an ansible 5 bug? | 21:26 |
| @clarkb:matrix.org | ianw: very strongly suspected yet. TLDR is that ansible 5 isn't respecting our ssh_connection.pipelining=true config so we are running all ansible 5 without pipelining | 21:27 |
| @clarkb:matrix.org | * ianw: very strongly suspected yes. TLDR is that ansible 5 isn't respecting our ssh\_connection.pipelining=true config so we are running all ansible 5 without pipelining | 21:27 |
| @iwienand:matrix.org | great find! would explain a lot | 21:28 |
| @clarkb:matrix.org | the only workaround with ansible 5 as is is to enable pipelining globally for all connections that support it. Which is potentially problematic for users | 21:29 |
| @clarkb:matrix.org | so we're going to push towards ansible 6 instead (which is what the next release brings) | 21:30 |
| @clarkb:matrix.org | at this point ansible 6 should be present in opendev. We can probably work to start converting the zuul tenant over | 21:32 |
| @clarkb:matrix.org | corvus: ^ is the best way to do that to just rip the bandaid off and fix what breaks? If so I guess after the release is made? | 21:32 |
| @jim:acmegating.com | Clark: i'd like to wait until we're happy enough with opendev that we actually make the 640 release (tomorrow) before we approve all the things | 22:05 |
| @jim:acmegating.com | mostly so it's easier to do a brown-bag release if we need to | 22:05 |
| @jim:acmegating.com | Clark: seems like we can do a test change to run ansible 6... then yeah, maybe after the release change the zuul tenant default | 22:06 |
| @clarkb:matrix.org | ack. I've just been +2'ing things. Now got the giant console datalist chnage in front of me | 22:06 |
| @clarkb:matrix.org | corvus: ianw the deeplinking behavior was the thing that is difficult to test right? And iirc ianw tested that locally? | 22:08 |
| @jim:acmegating.com | yep, that's my recollection. | 22:08 |
| @iwienand:matrix.org | yeah i did test the modal comes up when i pasted in a permalink from my localhost:3000 site. i mean, someone else should check, but it worked for me :) but also none of those bits about matching the path have really changed either | 22:09 |
| @jim:acmegating.com | i'm happy with a self-certification on that :) i agree it seems unlikely to break -- just didn't want it to be missed. | 22:10 |
| @jim:acmegating.com | Clark: https://review.opendev.org/854046 and child are the deprecation removal changes needed after 640 and before 700 | 22:19 |
| @jim:acmegating.com | the ansible2 removal change still needs to be written | 22:19 |
| @clarkb:matrix.org | ianw: some questions/comments on https://review.opendev.org/c/zuul/zuul/+/854556 | 22:38 |
| @jim:acmegating.com | Clark: re your last comment: unless we're sure that gets optimized out (a compiler *could* do that, i don't know if it *does*) then i agree that's worth an update). | 22:50 |
| @clarkb:matrix.org | corvus: some languages like haskell will lazily evaluate things like that only when they are needed. It makes debugging fun and I doubt that the javascript engine in our brwosers does it, but it is possible | 22:52 |
| @jim:acmegating.com | modern JS JITs are so powerful i would not put anything past them :) | 22:54 |
| -@gerrit:opendev.org- Zuul merged on behalf of Alfredo Moralejo: [zuul/zuul-jobs] 838450: Update gpg key file for extras-common in CS9 https://review.opendev.org/c/zuul/zuul-jobs/+/838450 | 23:10 | |
| @iwienand:matrix.org | Clark: sure i can refactor a bit | 23:15 |
| @iwienand:matrix.org | Clark: if you have the sample site up and can find a better way to pad on your high dpi screen i'm all ears :) i think we have agreement from upstream to be able to pass a disable flag via props to the button, and then we can hide it with css -- but it will require an update to PF | 23:17 |
| @iwienand:matrix.org | https://github.com/patternfly/patternfly-react/issues/7950#issuecomment-1239895122 | 23:17 |
| @clarkb:matrix.org | ianw: to be fair this is all because I'm half blind and linux high dpi support is not great | 23:18 |
| @clarkb:matrix.org | It doesn't bother me too much and I'm assuming things are aligned on your displays | 23:18 |
| @clarkb:matrix.org | If I change my default zoom to 100% instead of 110% the problem persists so it isn't that | 23:19 |
| @iwienand:matrix.org | if you can inspect the padding box, currently it's setting marginRight: 'var(--pf-c-data-list__item-control--MarginRight)' | 23:20 |
| @iwienand:matrix.org | actually, it's mroe likely to be the | 23:21 |
| @iwienand:matrix.org | <span style={{display: 'inline-block', minWidth: '40px'}} | 23:22 |
| @clarkb:matrix.org | there are a bunch of empty divs after the script | 23:23 |
| @clarkb:matrix.org | but looking for that particular row now | 23:23 |
| @iwienand:matrix.org | possibly i could put a disabled button in there to pad it out. which is more or less what upstream would do, except not hacky | 23:23 |
| @iwienand:matrix.org | the other option is to rewrite the whole thing as a table :) but i think that just trades one problem of padding this out for a bunch of other problems, most of which i don't even know are problems yet :) | 23:24 |
| @clarkb:matrix.org | ianw: the item control thing for the drop down is 30x30 and the explicit shift is 40x28 | 23:27 |
| @clarkb:matrix.org | I think its that 10 pixel difference that causes the problem | 23:27 |
| @iwienand:matrix.org | interesting; i wonder why it doesn't have minWidth:40px like mine does | 23:28 |
| @iwienand:matrix.org | is there any clue what css variable it's using? | 23:28 |
| @clarkb:matrix.org | ianw: the thing you added is margin-right: var(--pf-c-data-list__item-control--MarginRight); | 23:29 |
| @iwienand:matrix.org | i added a <span minwidth=40px> to simulate the button and then set the margin-right on that | 23:30 |
| @clarkb:matrix.org | The other thing is margin-right: var(--pf-c-data-list__item-control--MarginRight); | 23:30 |
| @clarkb:matrix.org | so they are both using the same margins but with different sizes of internal objects? | 23:30 |
| @clarkb:matrix.org | the actual toggle is 36x27 | 23:31 |
| @clarkb:matrix.org | Can we maybe wrap your span in class="pf-c-data-list__item-control" to have it render similarly? | 23:34 |
| @clarkb:matrix.org | supposedly the minimum for ^ is 30px | 23:34 |
| @clarkb:matrix.org | if I change your 40px to 30/31/32px it is close (super hard to tell if exact on this display | 23:36 |
| @iwienand:matrix.org | yeah, that might have the width ... i'm having a poke now | 23:36 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: | 23:44 | |
| - [zuul/zuul] 855293: Add tracing tutorial https://review.opendev.org/c/zuul/zuul/+/855293 | ||
| - [zuul/zuul] 855096: Tracing: implement span save/restore https://review.opendev.org/c/zuul/zuul/+/855096 | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed on behalf of Simon Westphahl: | 23:44 | |
| - [zuul/zuul] 856523: Add span for builds and propagate via request https://review.opendev.org/c/zuul/zuul/+/856523 | ||
| - [zuul/zuul] 857421: Trace merge requests and merger operations https://review.opendev.org/c/zuul/zuul/+/857421 | ||
| -@gerrit:opendev.org- Ian Wienand proposed: | 23:44 | |
| - [zuul/zuul] 857793: web: refactor console item generation https://review.opendev.org/c/zuul/zuul/+/857793 | ||
| - [zuul/zuul] 857794: web: better non-expandable console padding https://review.opendev.org/c/zuul/zuul/+/857794 | ||
| @iwienand:matrix.org | Clark: could you try 857794? that may be a better thread to pull on | 23:44 |
| @clarkb:matrix.org | I'll look just as soon as I finish the logo chang ereview. Related to the logo change review is this the sort of thing I should give as feedback to the foundation design team? | 23:46 |
| @clarkb:matrix.org | something like logos shouldn't contain extra whitespace around them to make ease of web dev simpler? | 23:46 |
| @jim:acmegating.com | swest: tristanC ^ that's a fairly significant rework of the tracing api -- it sets a global tracer and relies on that so that we don't have to pass around as many objects, and also moves even closer to using the otel api as designed. that's a bit more rework of existing changes than i normally like to do, but i think that's the best way to review it (adding the api all at once so it can be reviewed as a unit rather than a series of changes that each undo the last). i squashed several changes together to accomplish that. | 23:47 |
| @jim:acmegating.com | Clark: i don't think that file came directly from the foundation design team | 23:47 |
| @clarkb:matrix.org | corvus: ah | 23:47 |
| @clarkb:matrix.org | ianw: heh I just realized I need to wait for CI to build the previe wsite for me. But ya I'll check the result sof that change in my browser | 23:48 |
| @iwienand:matrix.org | > <@jim:acmegating.com> Clark: i don't think that file came directly from the foundation design team | 23:49 |
| in the original commit about the logo it says it came from https://www.openstack.org/brand/openstack-logo/logo-download/ and had its viewbox slightly adjusted. but where that came from i'm not sure | ||
| @iwienand:matrix.org | To me it seems logical to give the logos without any whitespace and let the users figure that out. but I bet someone has been paid big $$$ to figure out the exact amount of whitespace that the brand standards require :) But in the Zuul logo case I don't think it's an even padding; so it doesn't feel intentional | 23:51 |
| -@gerrit:opendev.org- Ian Wienand proposed: | 23:58 | |
| - [zuul/zuul] 857794: web: better non-expandable console padding https://review.opendev.org/c/zuul/zuul/+/857794 | ||
| - [zuul/zuul] 857795: web: console: make magnifying glass also clickable https://review.opendev.org/c/zuul/zuul/+/857795 | ||
| @iwienand:matrix.org | Clark: sorry -- new version may be better | 23:59 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!