| corvus | ++ | 00:03 |
|---|---|---|
| *** tosky has quit IRC | 00:14 | |
| *** ikhan has joined #zuul | 00:35 | |
| *** webknjaz has quit IRC | 01:01 | |
| *** webknjaz has joined #zuul | 01:02 | |
| *** Goneri has quit IRC | 01:29 | |
| ianw | sean-k-mooney/fungi : https://86c241e7a0127c842c18-9c306790d1324e3f8d887bee10e35650.ssl.cf5.rackcdn.com/763281/1/check/zuul-build-dashboard-opendev/2018ad2/npm/html/t/openstack/build/00ff397ca5374a0b9441036d43ee3416/log/controller/logs/screen-n-cpu.txt | 02:03 |
| ianw | should be a deep link to the stie with the fix at the file we were looking at; probably worth a good double check to make sure | 02:03 |
| *** hamalq has quit IRC | 02:09 | |
| *** piotrowskim has quit IRC | 02:20 | |
| fungi | "not found" | 02:34 |
| fungi | i've never had much luck deep-linking into the draft builds | 02:35 |
| clarkb | ya you have to got to npm/html/ then search for the build uuid 00ff397ca5374a0b9441036d43ee3416 in the builds list | 02:36 |
| clarkb | then you should be able to navigate to that path | 02:36 |
| fungi | yeah, that's what i'm (slowly) doing | 02:36 |
| *** ikhan has quit IRC | 02:39 | |
| *** ikhan has joined #zuul | 02:39 | |
| fungi | definitely seeing the lines which lack severities now | 02:40 |
| fungi | still have to scroll all the way to the bottom to find the horizontal scrollbar though, i guess that's not fixed in master yet | 02:41 |
| ianw | yeah ,this was independent of the scrolling issues | 02:46 |
| ianw | my opinion on that is that we should wrap the HTML view of the logs, and leave fixed width up to a raw view of the logs. to me, that's consistent with what i've checked github and gitlab do by default. but ... i many be a minority opinion on that | 02:48 |
| *** zenkuro has quit IRC | 02:59 | |
| *** bhavikdbavishi has joined #zuul | 03:02 | |
| *** bhavikdbavishi1 has joined #zuul | 03:11 | |
| *** bhavikdbavishi has quit IRC | 03:13 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 03:13 | |
| *** bhavikdbavishi has quit IRC | 04:00 | |
| *** bhavikdbavishi has joined #zuul | 04:01 | |
| *** bhavikdbavishi has quit IRC | 04:27 | |
| *** bhavikdbavishi has joined #zuul | 04:27 | |
| *** vishalmanchanda has joined #zuul | 04:30 | |
| *** ianychoi has joined #zuul | 04:36 | |
| *** evrardjp has quit IRC | 05:33 | |
| *** evrardjp has joined #zuul | 05:33 | |
| *** bhavikdbavishi1 has joined #zuul | 05:55 | |
| *** bhavikdbavishi has quit IRC | 05:56 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 05:56 | |
| *** Phoenikzz has joined #zuul | 06:01 | |
| *** saneax has joined #zuul | 06:10 | |
| *** hamalq has joined #zuul | 06:25 | |
| *** sanjayu_ has joined #zuul | 06:37 | |
| *** saneax has quit IRC | 06:40 | |
| *** slaweq has joined #zuul | 07:01 | |
| *** bhavikdbavishi has quit IRC | 07:14 | |
| *** bhavikdbavishi has joined #zuul | 07:35 | |
| *** jpena|off is now known as jpena | 07:42 | |
| felixedel | corvus, ianw: Could you give me a short update on the current logviewer page issue? I find it hard to follow in the IRC history. Just one small comment: The state after https://opendev.org/zuul/zuul/commit/b4b5b9fd58dd5d5aeb9d32331e4882d7ef5faf06 was merged is the one where the layout is completely broken due to the scrolling "fix". So we shouldn't use that as reference. | 07:46 |
| *** bhagyashris|off is now known as bhagyashris | 07:55 | |
| *** hashar has joined #zuul | 08:00 | |
| *** rpittau|afk is now known as rpittau | 08:03 | |
| *** sugaar has joined #zuul | 08:04 | |
| *** jcapitao has joined #zuul | 08:09 | |
| *** hamalq has quit IRC | 08:28 | |
| avass | tobiash: memory usage is still stable just below 3Gb, maybe disconnecting from zookeeper was causing those memory issues | 08:30 |
| *** nils has joined #zuul | 08:30 | |
| *** tosky has joined #zuul | 08:44 | |
| tobiash | avass: could be, we have fixed one memory issue in that area but since zk disconnects are not really handled at all there might be more memleaks lingering around when a zk disconnect happens | 09:04 |
| *** hamalq has joined #zuul | 09:29 | |
| *** holser has joined #zuul | 09:33 | |
| *** hamalq has quit IRC | 09:34 | |
| openstackgerrit | zbr proposed zuul/zuul master: WIP: Improve summary build layout https://review.opendev.org/763333 | 10:05 |
| *** hamalq has joined #zuul | 10:07 | |
| *** hamalq has quit IRC | 10:11 | |
| *** hamalq has joined #zuul | 10:27 | |
| openstackgerrit | Merged zuul/zuul master: web: remove optional match on systemd regex https://review.opendev.org/763281 | 10:30 |
| *** hamalq has quit IRC | 10:32 | |
| *** bhavikdbavishi has quit IRC | 10:54 | |
| *** hamalq has joined #zuul | 11:09 | |
| *** hamalq has quit IRC | 11:14 | |
| *** hamalq has joined #zuul | 11:30 | |
| *** zenkuro has joined #zuul | 11:31 | |
| *** hamalq has quit IRC | 11:35 | |
| *** hamalq has joined #zuul | 11:51 | |
| *** bhavikdbavishi has joined #zuul | 11:52 | |
| *** bhavikdbavishi1 has joined #zuul | 11:55 | |
| *** hamalq has quit IRC | 11:56 | |
| *** bhavikdbavishi has quit IRC | 11:57 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 11:57 | |
| openstackgerrit | Slawek Kaplonski proposed zuul/zuul-jobs master: [multi-node-bridge] Add script to configure connectivity https://review.opendev.org/762650 | 12:02 |
| *** hamalq has joined #zuul | 12:12 | |
| *** hamalq has quit IRC | 12:16 | |
| *** wuchunyang has joined #zuul | 12:18 | |
| *** wuchunyang has quit IRC | 12:22 | |
| sshnaidm | can parent job be paused with zuul_return in post playbook? Works for me only for run playbooks | 12:28 |
| *** holser has quit IRC | 12:29 | |
| *** jcapitao is now known as jcapitao_lunch | 12:30 | |
| *** hamalq has joined #zuul | 12:34 | |
| *** jpena is now known as jpena|lunch | 12:37 | |
| *** hamalq has quit IRC | 12:39 | |
| *** zenkuro has quit IRC | 12:42 | |
| *** zenkuro has joined #zuul | 12:43 | |
| *** ikhan has quit IRC | 12:53 | |
| *** rlandy has joined #zuul | 13:01 | |
| *** rlandy is now known as rlandy|rover | 13:01 | |
| tobiash | sshnaidm: it can only pause after the run phase | 13:03 |
| sshnaidm | tobiash, ack, thanks | 13:04 |
| *** jcapitao_lunch is now known as jcapitao | 13:07 | |
| *** rpittau is now known as rpittau|brb | 13:16 | |
| tobiash | zbr, ianw, corvus: the ansi render change has a huge performance impact in our system. We have some jobs that produce ~40MB of json log and with that rendering the build page increased from 5s to 60s | 13:18 |
| tobiash | I also tried the re-ansi patch from tristanC (https://review.opendev.org/762759) and that fails with that job with 'too much recursion' | 13:19 |
| zbr | hmm... | 13:20 |
| zbr | is this rendering happening all at once when the json is loaded or when it is displayed. | 13:20 |
| zbr | something is really fishy because I am sure that 40mg json would not load in 5s even without ansi. | 13:21 |
| zbr | tobiash: any link to such a job? | 13:21 |
| tobiash | in our private deployment | 13:21 |
| zbr | that reminds me of the moment some people were complaining about jenkins console not loading, when they did cat the logs to stdout... | 13:22 |
| tristanC | tobiash: is there ansi code in the json? | 13:23 |
| zbr | tristanC: stdout/stderr lines are inside the json. that is where zuul is displaying them. | 13:23 |
| tobiash | not that I know of, the failing task is a huge bazel log which has ~20MB | 13:23 |
| zbr | but if someone did not disable stdout on ansible task that can produce this amount of data, it will still f*** the client, regardless if is ansi or not. | 13:24 |
| zbr | how much stdout can someone produce until you count it as an DDOS for the CI system? | 13:24 |
| tristanC | tobiash: I can have a look at removing the recursion from re-ansi | 13:24 |
| zbr | tristanC: i guess that tobiash can produce you a test/benchmark suite :D | 13:25 |
| avass | tobiash: I think we might have the same problem | 13:26 |
| zbr | i think it should be easy to make the ANSI rendering optional, at least user could enable/disable it. could prove a workaround and easy way to debug. | 13:28 |
| zbr | in fact we could make the renderer a dropdown and allow multiple behaviors. | 13:28 |
| tobiash | we could also skip it if the output is above a certain threshold | 13:29 |
| tristanC | zbr: i also found that react-ansi fold is not working on https://zuul.opendev.org/t/zuul/build/8dfc41d53fc04e79ba6eb047ff935a14/console | 13:29 |
| avass | yep that 8Mb log took a long time to render | 13:29 |
| zbr | ouch,... my Chrome is on its knees | 13:30 |
| zbr | clearly we need to do something about it. not acceptable | 13:31 |
| *** jpena|lunch is now known as jpena | 13:32 | |
| zbr | i wonder if this has anything to do with extra code generated for the line numbers. | 13:32 |
| zbr | tristanC: do you know how to make a benchmaker for this case alone? it should be relevant as a worst-case scenario. (8MB). | 13:34 |
| avass | surprisingly firefox seem to handle it better than chrome | 13:34 |
| pabelanger | zuul-maint: morning, I would like to request 0.0.1 tag of zuul-regsitry. I understand a lot of folks might be using the docker image, currently we are still consuming pypi artifacts. The release would help us run zuul-registry (hopefully) today | 13:35 |
| *** holser has joined #zuul | 13:35 | |
| *** zenkuro has quit IRC | 13:37 | |
| *** zenkuro has joined #zuul | 13:37 | |
| avass | and ram usage spikes a lot | 13:39 |
| *** hamalq has joined #zuul | 13:40 | |
| zbr | huge difference between chrom (30s) and firefox (11s) | 13:41 |
| zbr | 11s would probably count as acceptably for me, for this size, but clearly not 30. | 13:41 |
| zbr | what is worse is that chrome become sluggish even after loading it, trying to select lines and you will see how slow is moving | 13:42 |
| zbr | this makes me think that it may be related to number of DOM nodes. | 13:42 |
| avass | 43 seconds for that 8mb log on firefox for me, lots of color in it | 13:43 |
| *** hamalq has quit IRC | 13:45 | |
| zbr | current implementation is also poor as it does use inline styles instead of classes. | 13:45 |
| *** Goneri has joined #zuul | 13:47 | |
| avass | 4min10 seconds on chrome | 13:47 |
| zbr | tristanC: wow! you implementation loaded it in ~3-4s | 13:49 |
| zbr | clearly it does not produce line numbers, but i could live without them, at least for a while. | 13:50 |
| tristanC | zbr: that's suprising, the code is not even optimized. I think it could load even faster by replacing the recursion with an accumulator | 13:54 |
| zbr | i may be missing something as I saved the file and is only ~1MB, and my expection was to be much bigger. | 13:58 |
| fungi | would this be impacting the log view too, or just the summary/console views? | 14:00 |
| zbr | afaik, we did not touch the log-view, yet. | 14:02 |
| corvus | tobiash, zbr, fungi, tristanC: i think we should go ahead and revert the ansi patch and continue working on this in review; we know that the current state is a performance regression from folks, and additionally, if we are going to release v4 in a couple of weeks, i don't want to release it with a half-implementation of ansi (ie, we need all the issues i raised on the change to be addressed) | 14:06 |
| avass | Yep it's a bit unusable for a lot of people here at the moment, I'm surprised no one told us | 14:07 |
| corvus | felixedel: of course, sorry -- i think the main problem with the log viewer page right now is that you can't use the left/right keyboard keys to scroll horizontally (but i believe you could in the distant past) | 14:08 |
| corvus | i'll prep a revert | 14:08 |
| zbr | i am afraid that this will make the adoption impossible for two reasons: nobody has full ANSI implementation, nobody, and second: that seems to be a problem only on very high log sizes. | 14:08 |
| corvus | zbr: i'm not sure i understand 'nobody has full ansi implementation' | 14:09 |
| zbr | there is not one ANSI specification, ANSI itselt has a huge number of extensions, variations and terminal behavior varies a lot from one terminal to another. | 14:09 |
| zbr | saying "full ANSI implementation" is adding an impossible to achieve goal. | 14:10 |
| zbr | one could always find something that will not work | 14:10 |
| corvus | zbr: oh, are you quoting me? that's not what i meant by "full ansi implementation". i meant that the implementation within zuul needs to be complete. for example, support for the log viewer page is missing. | 14:11 |
| zbr | maybe we should start by setting some measurable goals, including performance. | 14:11 |
| zbr | i propose something else: making the ANSI renderer configurable. | 14:12 |
| corvus | i think simple ansi color support is a decent start. | 14:12 |
| corvus | zbr: i don't think configurable is a good solution here | 14:12 |
| zbr | like we have the auto-reload | 14:12 |
| zbr | what i am trying to explain is that without testing it in the wild we will not be able to find bugs. like the performance ones reported recently. | 14:13 |
| corvus | at least, not if the only reason to do it is because the code isn't performant. | 14:13 |
| corvus | zbr: i'm saying it can be tested while in review | 14:13 |
| corvus | and all of the issues i raised can be addressed and tested while in review | 14:13 |
| avass | zbr: those aren't very large logs for us :) | 14:13 |
| corvus | the whole point of zuul's workflow is you don't have to merge broken changes just to test them | 14:13 |
| fungi | yeah, i'll admit when i +2'd it i only performed a cursory check of the draft render to make sure it actually did interpret the color sequences, i didn't think to try benchmarking it against various build results | 14:14 |
| corvus | so anyway, we found some issues, we'll back it out, and fix them, and then merge it | 14:14 |
| zbr | tristan patch does not seem to have the performance issue: https://review.opendev.org/#/c/762759/ | 14:16 |
| *** bhavikdbavishi has quit IRC | 14:17 | |
| zbr | i think that before doing more changes we should agree on what kind of behavior we want and what would be supported or not. | 14:19 |
| zbr | first: a maximum limit on number of lines to render, that applies to pure text also, even without ANSI. | 14:19 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Revert "Enable ANSI rendering via react-ansi" https://review.opendev.org/763380 | 14:20 |
| corvus | zbr: "no limit" is my answer to that. | 14:20 |
| zbr | someone i have the impression that some try to find any kind of reasons to revert ANSI related changes. | 14:22 |
| zbr | we know very well that such a change can have a performance/memory impact, is normal. I asked to set some reasonable limits for evaluating what would be considered an acceptable cost or not. | 14:22 |
| zbr | or else anyone can produce a DDOS sample change just to prove that in some cases it does not work. | 14:23 |
| corvus | zbr: i thought you just suggested a limit to the number of lines that zuul renders in general. | 14:23 |
| corvus | zbr: i didn't know you were asking for a limit for a specific test case. | 14:24 |
| zbr | corvus: not necessarly, i am sure you will find some kind of limit logical, if I feed your browser 200mb .... you may not be happy. | 14:24 |
| corvus | zbr: i actually don't care | 14:24 |
| tobiash | I think a limit where above we fall back to the old rendering would be ok for me | 14:24 |
| zbr | but we can easily make a decision to fallback to plain if count(lines)>5k. | 14:25 |
| corvus | zbr: i think that ansi rendering should be nearly as fast as the current rendering | 14:25 |
| zbr | corvus:tell this to those that created alacritty terminal. | 14:26 |
| corvus | zbr: if we find the speed acceptable, that's fine. i just don't think there should be a limit to the usefulness of zuul as a log viewer. it's on the user not to produce or view logs larger than their browser can handle. just like today. if you want to load a 500mb file, we'll load it and render it. | 14:26 |
| zbr | for the moment I think setting a fallback to plain is very easy to implement and should sort the loading issue. | 14:27 |
| corvus | i don't think that's how we should start | 14:28 |
| zbr | i am not sure why tristanC patch has a -2 on it. | 14:28 |
| corvus | i think you should start with "make it as fast as it can be" | 14:28 |
| corvus | and then, when you have achieved that, if there are still issues, we can look into a cutoff or something | 14:28 |
| * zbr someone needs a prize for setting goals that are not measurable | 14:28 | |
| corvus | zbr: it seems whenever you say 'someone' you are referring to me obliquely. that seems rude. | 14:29 |
| corvus | zbr: i'm trying to provide actionable advice to you | 14:29 |
| zbr | while i do not want to offend anyone, i personally find "make it as fast as it can be" requirement offensive. | 14:30 |
| corvus | zbr: how is that offensive? tristanC has suggested a method which is faster than the current system and believes that it can be made faster. so it sounds like investigating improving the performance is a route which could lead somewhere. | 14:31 |
| tristanC | fwiw i have a test case that reproduce the performance issue reported by tobiash here: https://softwarefactory-project.io/r/20109 | 14:31 |
| corvus | so my suggestion is that you work on that to improve it. i'm sorry i don't have hard criteria for what would be an improvement, but it's also not my responsibility as a reviewer to create detailed specifications for your changes. but i think now that you've been made aware that performance is a concern, you should take that feedback, measure performance, evaluate whether it can be improved. | 14:33 |
| zbr | corvus: would an increase of 20-30% load time seen as acceptable for under 20.000 lines? | 14:38 |
| zbr | i do agree that 30s instead of 12s is not acceptable and this is what i seen on Chome. I do have a reference file to look at now. | 14:38 |
| corvus | tobiash, avass: ^? thoughts | 14:39 |
| *** yolanda has quit IRC | 14:40 | |
| tobiash | I'd be ok with a penalty of 20-30% (with our 40MB logfile) | 14:41 |
| tobiash | but I'd also be ok to not ansi-render a log >5MB or so | 14:41 |
| avass | I think our users wants the ansi rendering and I agree that not producing too large log files to begin with is better, but I'm not sure if we can directly influence that in this case | 14:41 |
| zbr | thank everyone, these numbers are very important | 14:41 |
| tobiash | but if we manage a penalty of lower than 20% or so I don't think there is a need to make such a switch | 14:42 |
| avass | I think it should work, at least as a starting point since 40s/2m,10s is way too much as it is at the moment | 14:42 |
| avass | those logs are probably going to grow as well, but I'll try to see if we can reduce the sizes of those logs or at least split it into multiple tasks somehow | 14:43 |
| corvus | i'd like to either avoid the switch or make it as high as possible -- there's no correlation between log size and ansi usage (if anything, ansi logs are likely to be bigger!) -- it's purely a tradeoff for technical implementation reasons, so if we want to improve the ux, we should try to make it universal (within our ability to do so) | 14:44 |
| avass | I also have way too much to do at the moment :( | 14:45 |
| corvus | so maybe we aim for 20-30% penalty, and then find out at what size it gets very slow (very slow == 5 seconds? 10?) then add a cutoff for that size | 14:45 |
| corvus | by cutoff i mean disable ansi | 14:46 |
| avass | I think it would help if it could render it progressively somehow | 14:46 |
| corvus | that's something to explore; with that we need to keep the 'ctrl-f' use-case in mind, but it's possible that "scroll to the bottom to render everything before searching" is a reasonable compromise | 14:47 |
| zbr | corvus: i totally agree with keeping the search in page usable, very important feature. | 14:49 |
| zbr | I need feedback on this change (left is new, look around space and guid): https://sbarnea.com/ss/Screen-Shot-2020-11-19-14-49-10.png | 14:50 |
| zbr | if you click the guids and copy text it will copy the entire text, ellipses are added by browser, the full value is inside the page. | 14:51 |
| *** rpittau|brb is now known as rpittau | 14:51 | |
| avass | it doesn't have to block it from rendering until the user scrolls down, if it could split the log into chunks and render each chunk in order I think it could feel more responsive | 14:52 |
| avass | but that might be confusing since ctrl-f won't work until all chunks are rendered | 14:53 |
| corvus | zbr: lgtm | 14:54 |
| corvus | avass: maybe with a spinner that would be ok | 14:54 |
| corvus | zbr: you're still planning on moving the times over too, right? | 14:55 |
| zbr | this will allow me to move the time fields to first columns without making it worse than it is now). | 14:55 |
| corvus | ++ | 14:55 |
| zbr | yes, but I will move the time when I move the artifacts. | 14:55 |
| corvus | makes sense | 14:55 |
| zbr | i am trying to do them in easy to review parts | 14:56 |
| corvus | i think this could all be one change, but whatever works for you | 14:56 |
| zbr | i was bit disappointed that my A+B proposal around the branch and pipeline were not popular as they would have removed two rows from view. | 14:56 |
| zbr | and I do think that the colored label is more visible than a list item anyway. | 14:57 |
| zbr | maybe I will make a POC to show it after I do the rest of them. | 14:57 |
| corvus | sounds reasonable | 14:57 |
| zbr | the most visible part of the page is the green line | 14:57 |
| zbr | (well red for failures) | 14:57 |
| zbr | we could even use colors to distinguish between pipelines. same kind of approach could be used for displaying the branch. | 14:58 |
| zbr | every branch that is not master could get a different color, making hard to confuse something run on default branch (master/main) from somethign else. | 14:59 |
| openstackgerrit | zbr proposed zuul/zuul master: Improve summary build layout https://review.opendev.org/763333 | 15:00 |
| *** bhavikdbavishi has joined #zuul | 15:01 | |
| tristanC | tobiash: fwiw i think https://softwarefactory-project.io/r/20109 should fix the recursion error, i'll cut a 0.2.0 release shortly | 15:04 |
| tobiash | tristanC: cool, then I can test it locally | 15:04 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul master: web: replace react-ansi component with re-ansi https://review.opendev.org/762759 | 15:09 |
| tristanC | tobiash: here you go^ | 15:09 |
| tristanC | it is still not optimized though, i just tweaked the recursion to use an accumulator and trigger the TCO | 15:11 |
| fungi | zbr: using only colors to differentiate is bad for accessibility, but if we're coupling color with text that seems acceptable | 15:12 |
| tobiash | tristanC: are you sure that this is right? https://review.opendev.org/#/c/762759/7..8/web/yarn.lock | 15:16 |
| tobiash | that disagrees with the package.json | 15:16 |
| tristanC | oops, forgot to commit the yarn lock | 15:16 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul master: web: replace react-ansi component with re-ansi https://review.opendev.org/762759 | 15:16 |
| *** tosky has quit IRC | 15:18 | |
| tobiash | tristanC: is that expected to not produce black console like background? | 15:20 |
| tristanC | tobiash: yep, this should be set by the parent component | 15:22 |
| tobiash | tristanC: so it actually looks like without the ansi patch if there is no ansi stuff in the text? | 15:22 |
| tristanC | and i'm not sure we should do black by default, it's less accessible than white background | 15:22 |
| *** tosky has joined #zuul | 15:22 | |
| tobiash | I just want to verify that my local test uses it ;) | 15:23 |
| fungi | the black background will probably be more appropriate with colorized text, unless the foreground palette is shifted to be distinguishable on it | 15:23 |
| tristanC | tobiash: it shouldn't interfer if there is no ansi code | 15:23 |
| tobiash | looks like it loads as fast as with no ansi | 15:24 |
| fungi | but i honestly don't care all that much which background color it ends up with as long as non-colorized text is readable, i don't personally expect to be looking at a lot of logs with embedded ansi color sequences | 15:24 |
| corvus | yeah, i suspect that a dark bg (probably not black) would be okay and a survey of the literature would be helpful | 15:25 |
| tristanC | fungi: a better palette would be a nice addition | 15:25 |
| *** tosky has quit IRC | 15:26 | |
| corvus | fungi: want to take a look at https://review.opendev.org/763380 ? | 15:26 |
| openstackgerrit | zbr proposed zuul/zuul master: Build summary: moved artifacts to middle column https://review.opendev.org/763395 | 15:27 |
| tristanC | corvus: https://ux.stackexchange.com/questions/53264/dark-or-white-color-theme-is-better-for-the-eyes has some link that indicate light background are better | 15:29 |
| tristanC | s/better/more accessible/ | 15:29 |
| tobiash | tristanC: I think I know why that renders so fast, it renders only the last line | 15:29 |
| corvus | tristanC: nice, looks like some good material linked there | 15:30 |
| tristanC | tobiash: oh my :-) | 15:31 |
| fungi | when i was younger i knew folks who swore "troglodyte mode" (turn off the lights, invert the terminal colors, wear sunglasses) significantly reduced eye strain, but i could never get used to it personally | 15:31 |
| avass | fungi: wait, with invert terminal color you mean a dark room with a super bright screen? | 15:37 |
| zbr | selling PC glasses was indeed a good business | 15:41 |
| openstackgerrit | David Moreau Simard proposed zuul/zuul master: DNM: Test zuul-stream-functional with up-to-date ara https://review.opendev.org/694622 | 15:49 |
| *** tosky has joined #zuul | 15:50 | |
| tobiash | corvus: looking at this ^ do we still use ara somewhere in zuul or is that obsolete with current zuul-web? | 15:52 |
| dmsimard | o/ I don't think we should merge that at all but I wanted to show someone what it would roughly look like | 15:53 |
| tobiash | dmsimard: no offense, that just reminded me about that topic ;) | 15:54 |
| dmsimard | the current implementation of ara in zuul is unfortunately outdated and it's since then diverged between 1.x and the zuul-web/zuul-json callback | 15:56 |
| zbr | once i finish the summary page changes, I will be very happy to focus on removing the popup from the console,... every time i click it it gives me motion sickness. | 15:58 |
| zbr | that should have being just an expand option, not a popup. | 15:58 |
| dmsimard | fwiw I see many opportunities to collaborate but I don't feel like ara should be "hardcoded" in zuul at all | 15:59 |
| tobiash | we get an error in our logs about ara in every playbook zuul executes: Ansible output: b"'ansible.plugins.callback.ara_record' has no attribute 'CallbackModule'" | 15:59 |
| tobiash | so I wonder if we should fix that or just remove ara from zuul | 15:59 |
| dmsimard | that's an odd error, there's no such thing as ansible.plugins.callback.ara_record | 16:00 |
| *** vishalmanchanda has quit IRC | 16:00 | |
| dmsimard | ara_record is in fact an action plugin which I don't believe zuul enables because it doesn't add the action plugins path to ANSIBLE_ACTION_PLUGINS | 16:01 |
| tobiash | ansible skips that plugin then, so nothing serious, but spams our logs | 16:01 |
| avass | I think it should be possible to configure ara with the ansible_callback section instead of hardcoding it | 16:01 |
| avass | maybe it needs an update to handle callbacks that aren't builtin though | 16:01 |
| dmsimard | avass: yeah, I've seen those bits and indeed, it's only for whitelisting callback plugins that ship with ansible | 16:02 |
| dmsimard | another issue (and a facepalm is appropriate here) is that the ara callback picks up config from the [ara] block, not [callback_ara] | 16:03 |
| tobiash | zuul explicitly adds the ara callbacks btw: https://opendev.org/zuul/zuul/src/branch/master/zuul/executor/server.py#L865 | 16:03 |
| dmsimard | that's part historical because that's how it was in 0.x and [callback_*] was not a thing in past ansible versions | 16:03 |
| avass | I think when this was last discussed the idea was to handle it when zuul had support for ansible collections | 16:04 |
| dmsimard | that's a rabbithole :) | 16:05 |
| dmsimard | part of the problem with collections is that they do not manage python dependencies | 16:05 |
| dmsimard | so if you have ansible plugins that require certain python deps to be installed, you need to manage the installation separately | 16:06 |
| pabelanger | execution environments from ansible are to fix python deps | 16:06 |
| pabelanger | they will also use bindep for OS package management | 16:06 |
| pabelanger | that is what I am testing now | 16:06 |
| dmsimard | pabelanger: realistically not everyone is going to be using execution environments though | 16:06 |
| avass | yeah I started working on it here: https://review.opendev.org/#/c/723071/6 and that would work the same the environment varaibles for python deps do | 16:06 |
| pabelanger | dmsimard: true, but moving forward that is basically what people want to support | 16:07 |
| dmsimard | moar containers | 16:08 |
| pabelanger | has anyone started work to enable 2.10 support in zuul jobs | 16:08 |
| pabelanger | err | 16:08 |
| pabelanger | zuul it self | 16:08 |
| pabelanger | the good news, if we wanted, we could ship our own stripped down ansible version now | 16:09 |
| pabelanger | but, that would be a lot of work to manage ansible | 16:09 |
| tobiash | pabelanger: wip: https://review.opendev.org/757452 | 16:09 |
| tobiash | not fully functional yet, feel free to take over if you want | 16:10 |
| avass | I think it would be reasonable to have a couple of default installed collections so it's easier to use 2.10 | 16:10 |
| pabelanger | avass: if you pip install ansible 2.10, you shouldn't need to manually install any collections | 16:11 |
| tobiash | ansible is ansible-base plus all the collections that were builtin earlier | 16:11 |
| pabelanger | right | 16:12 |
| tobiash | at least what I've understood | 16:12 |
| avass | pabelanger: I think I had to install the crypto collection | 16:12 |
| pabelanger | and ansible-base is becoming ansible-core | 16:12 |
| pabelanger | avass: yah, it is possible some collections are missing, depending on if they didn't get packaged in time | 16:12 |
| pabelanger | I'm not sure what got removed | 16:12 |
| tobiash | pabelanger: I think the 2.10 change still has problems with the plugin hooks since they can now be namespaced by the collection name | 16:13 |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul master: web: replace react-ansi component with re-ansi https://review.opendev.org/762759 | 16:13 |
| pabelanger | tobiash: yah, that is possible | 16:13 |
| tristanC | tobiash: i think i got this, just forgot to return the accumulator at the end of the document :-) fwiw here is the fix in 0.2.1 https://softwarefactory-project.io/r/#/c/20112/1/src/Ansi.re | 16:15 |
| openstackgerrit | Paul Belanger proposed zuul/zuul master: WIP: Support ansible 2.10 https://review.opendev.org/757452 | 16:16 |
| *** evrardjp has quit IRC | 16:17 | |
| tobiash | tristanC: ok, so that makes 3.5s with legacy, 60s with the current ansi and 7s with your ansi in my local testing | 16:19 |
| *** wuchunyang has joined #zuul | 16:19 | |
| tobiash | so penalty is ~100% with with my 40MB logfile acceptable for me at least | 16:20 |
| *** hamalq has joined #zuul | 16:20 | |
| avass | yep that sounds like it should work | 16:20 |
| avass | and could probably be improved by not rendering everything in one go | 16:20 |
| tobiash | I tested only loading the task summary so far though | 16:21 |
| tristanC | tobiash: the trick is that we can't simply look for escape sequence because js string are unicode, thus we have to call codePointAt for each characters. There may be some optimization to split the sequence (and avoid cutting through emoji are extended character set), but that may be too premature | 16:22 |
| tristanC | i can have a look to speed things up though | 16:23 |
| *** wuchunyang has quit IRC | 16:24 | |
| tobiash | console tab is 7s no ansi vs 14s new ansi vs browser freeze old ansi | 16:24 |
| *** hamalq has quit IRC | 16:25 | |
| tristanC | also the implementation supported nested escape sequence such as `<bold>text <red>text</red> text</bold>` which doesn't seem popular, dropping nested style could improve processing speed too | 16:26 |
| tristanC | supports* | 16:27 |
| fungi | yeah, ansi handling is hairy, i've implemented my own in python as well, you have to work on the stream as a bytestring because ansi escapes aren't valid utf-8 encoding so you can't just treat the data as a string, and also you need a state machine because it's possible for a sequence to set, say, red color and then never set back to unstyled, so you'd have an effectively unterminated span in the resulting | 16:27 |
| fungi | html. as a result you can't render later data in the stream without scanning everything which comes before it and remembering prior state | 16:27 |
| zbr | imho, nesting should not be supported, we should just flat it. | 16:28 |
| fungi | you can't really do 4-bit color if you don't support composition of escape codes, because it's implemented as 3-bit color plus a bold bit in separate values | 16:29 |
| fungi | i suppose you could assume that streams will always either set the color and bold within one escape sequence instead of in separate sequences, but i don't know that can be assumed | 16:31 |
| zbr | what i wanted to say is that the final outcome should not nest html elements. each time a new color starts, the previous one should end. | 16:32 |
| tobiash | tristanC: I've added my quick test results to your change | 16:32 |
| fungi | ansi color doesn't have concepts of "nesting" or "containers" because it's literally just commands saying "now change the text color to red" and "now change the background color to green" and "now set the font to bold" and "now unset all styles" | 16:33 |
| fungi | zbr: so if you have a sequence which says "set text color red" and then later "set font style bold (to get bright red instead of dark)" the original span with color=darkred should be closed and a new span with color=lightred should be started? | 16:34 |
| openstackgerrit | Merged zuul/zuul master: Revert "Enable ANSI rendering via react-ansi" https://review.opendev.org/763380 | 16:35 |
| zbr | fungi: when doing the conversion we clearly need to keep the state, but when a change occurs, we can just close the previous span and start a new one mentioning all styles that are to be applied to the new one. | 16:36 |
| zbr | this means that you may setup background color once per line in ANSI but the final html may have multiple sequential spans that will all mention the background color. | 16:36 |
| zbr | and each span will be a section that has the same style, closing it only when style changes. | 16:37 |
| fungi | also i'm not so sure about actually interpreting \E[1m as a literal sgml strong/bold but rather as a color shift | 16:38 |
| fungi | if you treat it as actually bold font then you only have 3-bit color not 4 | 16:38 |
| fungi | when you mention "once per line" are you assuming all color values will be reset at a newline, not carried over like the terminal would do? | 16:39 |
| fungi | or does setting a color in one line get carried through into subsequent lines until you change it? | 16:40 |
| avass | tobiash, tristanC: yep that's a lot faster. but the colors are a bit unreadable | 16:40 |
| zbr | avass: i already started working on replacing the colors | 16:41 |
| fungi | presumably that's doable with some simple classes in css | 16:41 |
| avass | and I see a couple of '[m[K [Knote: [m[K' that doesn't get rendered | 16:41 |
| fungi | avass: any idea what the raw byte representations for those are? | 16:42 |
| *** jpena is now known as jpena|off | 16:42 | |
| zbr | fungi: yes, css classes, but not as easy as you may think. | 16:43 |
| zbr | https://github.com/pycontribs/ansi2html/blob/master/tests/produce_headers.txt | 16:43 |
| zbr | as you can see there are lots of classes to generate | 16:44 |
| avass | fungi: something like this: 1b[01;36m1b[Knote: 1b[m1b[K | 16:52 |
| *** rpittau is now known as rpittau|afk | 16:52 | |
| avass | comparing to the current ansi it doesn't seem to do anything at all, maybe it's just ignored by react-ansi | 16:53 |
| fungi | huh, wonder what [K is doing in there | 16:54 |
| fungi | \E[K i mean | 16:54 |
| avass | yeah I'm not sure | 16:54 |
| fungi | K is 0x4b... | 16:55 |
| tristanC | avass: the default color can be improved indeed, here is how they are defined: https://github.com/softwarefactory-project/re-ansi/blob/master/src/Ansi.re#L35 | 16:56 |
| avass | tristanC: dim yellow became light magenta, and there's some magenta on red background as well :) | 16:58 |
| tristanC | avass: \E[K is not matched yet, it seems like this means to kill the rest of the line, and i wonder how we should interpret this in console log, only \E[1K is matched | 16:58 |
| fungi | avass: K in a csi sequence is "erase line" | 16:59 |
| tristanC | avass: i know they are not ideal :-) | 16:59 |
| fungi | er, "erase in line" | 16:59 |
| avass | uh I mixed taht up, the yelow stayed yellow but it's bright | 16:59 |
| fungi | avass: but if there is no value supplied before the K then it's erase from here to the end of the line | 16:59 |
| avass | fungi: yeah that's what I assumed, but I'm not sure why it would need to do that | 17:00 |
| *** hamalq has joined #zuul | 17:01 | |
| avass | oh I think cmake could be printing it's progress, then erasing that to overwrite with logs | 17:01 |
| fungi | avass: some command line tools do that to maintain an updating status line | 17:01 |
| avass | yeah exactly | 17:01 |
| fungi | to avoid scrolling the buffer | 17:01 |
| zbr | docker build new TUI is cool, but it may prove a nightmare for CI usage, see https://asciinema.org/a/373990 | 17:11 |
| zbr | but AFAIK, this should happen only on interactive terminals, and it should avoid rewriting for non ttys. | 17:12 |
| zbr | avass: fungi tristanC: my first two changes on the summary page are ready for review: https://review.opendev.org/#/q/topic:ux/summary+(status:open+OR+status:merged) | 17:17 |
| avass | zbr: I hope that it shows more lines if there's an error | 17:19 |
| avass | and to be honest I feel more comfortable if it didn't hide loglines like that | 17:20 |
| avass | zbr: I have a very strong feeling people are not going to understand that you can still copy the entire uuid if they're hidden like that | 17:22 |
| zbr | We cam easily add a tooltip hint. My plan is to add a button near it once I get a response from upstream | 17:23 |
| zbr | avass: see https://github.com/patternfly/patternfly-design/issues/956 | 17:24 |
| avass | I think it could work if there was a button to copy | 17:24 |
| zbr | I tried adding the copyclipboard element but is too bulky. | 17:24 |
| avass | zbr: can you clarify 'reduce spacing between list items'? | 17:25 |
| zbr | is visible in screenshot, it removes extra spacing between them in order to make total height lower | 17:26 |
| avass | oh there was a screenshot | 17:26 |
| zbr | in first comment, i did not want to include personal links into commit message | 17:27 |
| zbr | imho we do not need to document the copy, anyone that did a double click and copy will see the full text. | 17:28 |
| avass | I like the first change, except that I expect that some people are going think that you can copy the entire uuid is unintuitive. | 17:29 |
| avass | I feel like the second change makes the information a bit too dense but maybe it's because I'm not used to it | 17:29 |
| avass | but that's just all the information on the left hand side, having the artifacts visible is great | 17:31 |
| avass | I think I would have swapped the artifacts and the information on the right hand side | 17:32 |
| *** jcapitao has quit IRC | 17:33 | |
| corvus | i think we need the clipboard copy button; i agree otherwise it's doubtful people will understand that the whole thing is there | 17:38 |
| corvus | of course, copying the uuid is kind of an advanced user workflow; maybe it's not important? | 17:40 |
| avass | that's true | 17:41 |
| pabelanger | corvus: clarkb: I looked into nodepool release notes and think we are safe to tag afe86884d291cf873b855a76e84ad209e0560ab2 as 3.14.0. Looking to pick up fixes for https://review.opendev.org/752724/ and recent nodescan changes for FIPs mode | 17:41 |
| avass | in that case I would probably but them below the timestamps or somewhere else less important, but yet easily accessible | 17:42 |
| corvus | pabelanger: thanks; i can probably do that in a few hours | 17:42 |
| pabelanger | corvus: cool, thanks. Would that be the same for zuul-registry 0.0.1? | 17:42 |
| corvus | pabelanger: i reckon so -- is everything set up for that now? | 17:43 |
| pabelanger | corvus: it should be yes | 17:43 |
| corvus | k | 17:43 |
| pabelanger | corvus: actually, do we also want to tag docker image too? | 17:43 |
| pabelanger | or just latest | 17:43 |
| corvus | pabelanger: i think yes tag docker too | 17:44 |
| pabelanger | kk | 17:44 |
| pabelanger | let me get that change up | 17:44 |
| corvus | k. biab. | 17:44 |
| *** hamalq has quit IRC | 17:45 | |
| *** hamalq has joined #zuul | 17:45 | |
| openstackgerrit | Paul Belanger proposed zuul/zuul-registry master: Add upload-docker-image release job https://review.opendev.org/763419 | 17:47 |
| pabelanger | corvus: ^should be right, copypasta from zuul / nodepool job | 17:48 |
| pabelanger | corvus: also for your review queue, I was hoping we could support something like https://review.opendev.org/761756/ in zuul. For zuul.a.c, we often have to manually delete tags that upstream repos manually delete or worst retag with different sha1 | 17:51 |
| fungi | just a heads up, for gerrit users here who aren't following repo-discuss, all gerrit versions 2.14 and later have urgent security updates published in the past 24 hours: https://groups.google.com/g/repo-discuss | 17:53 |
| clarkb | later meaning >=2.14 | 17:55 |
| *** hamalq has quit IRC | 17:56 | |
| *** hamalq has joined #zuul | 17:56 | |
| *** sanjayu_ has quit IRC | 18:08 | |
| corvus | swest: why did you abandon 761756? | 18:18 |
| corvus | pabelanger: ^ | 18:18 |
| pabelanger | unsure right now, was going to ask to restore it if you were okay with approach | 18:19 |
| corvus | pabelanger: see comment on 419 | 18:20 |
| pabelanger | corvus: good point | 18:21 |
| openstackgerrit | Paul Belanger proposed zuul/zuul-registry master: Add upload-docker-image release job https://review.opendev.org/763419 | 18:22 |
| *** bhavikdbavishi has quit IRC | 18:30 | |
| corvus | tristanC: ^ can you +3 that? | 19:02 |
| tristanC | corvus: i left a comment as i can't tell what the jinja/yaml is actually doing | 19:10 |
| corvus | tristanC: i wrote that :( it's in all the zuul repos. the comment above tries to explain what it does. | 19:10 |
| tristanC | corvus: oh so the &imagetag anchor is used in another file perhaps? | 19:12 |
| pabelanger | yah, left comment. It is copypasta | 19:12 |
| corvus | tristanC: oh! i get it, sorry, yeah we don't actually need the anchor there | 19:12 |
| corvus | it's in the zuul/zuul one because it is used in multiple places | 19:12 |
| corvus | but it's a singleton here, so the anchor is not strictly necessary | 19:13 |
| corvus | i see now you have 2 issues really | 19:13 |
| corvus | so yeah: a) the anchor isn't necessary, i could take it or leave it here. b) the jinja is ugly: i don't disagree, but i also don't want to drive changing it right now because it's well established and we know it works and there's a comment above explaining it. | 19:14 |
| tristanC | alright, i didn't realized that was copypasta, perhaps a note to indicate where this is coming from would be useful for future maintainance | 19:14 |
| corvus | ++ | 19:15 |
| corvus | 'this cantrip is used in all zuul repos' or something | 19:15 |
| openstackgerrit | Mohammed Naser proposed zuul/zuul-jobs master: added lgtm with basic docs https://review.opendev.org/763428 | 19:15 |
| mnaser | corvus, pabelanger, tobiash and other github-interested folks, i added a scaffolding of lgtm that we setup in our zuul here ^ | 19:16 |
| mnaser | it works fine for us right now except comments inside code review are not parsed (so only real comments) | 19:16 |
| mnaser | and counts from most recent commit, etc | 19:16 |
| mnaser | it doesnt have much testing, and the docs are minimal at best. i know folks were interested in using something like this, i am ENOTIME to actually shape it into something shippable | 19:17 |
| mnaser | but anyone can feel free to take over the patch and land it to zuul-jobs (or make it more of a native zuul feature) or whatever y'all think is best :) | 19:17 |
| pabelanger | mnaser: how does lgtm enforce number of reviewers? Is that config at app level per repo? | 19:17 |
| mnaser | (sorry, didn't mean to do the whole "throw code across the wall" but i am quite sucked into a few things but this is better than nothing) | 19:18 |
| mnaser | pabelanger: it does not actually, it's similar to gerrit's "trusted system" | 19:18 |
| mnaser | the same way anyone can single approve inside gerrit, you can do inside this | 19:18 |
| corvus | mnaser: thanks -- this way at least it's there if someone wants to pick it up, or maybe you can get back to it in $time :) | 19:18 |
| pabelanger | mnaser: ack, that could be handled at branch protection level too | 19:18 |
| pabelanger | but not sure how lgtm app plays with that | 19:19 |
| mnaser | pabelanger: correct -- ah one more thing i should have added is you need branch protection too seutp | 19:19 |
| pabelanger | but, thanks! | 19:19 |
| mnaser | its not even an app, it purely is a pipeline | 19:19 |
| pabelanger | I will hack on it for sure | 19:19 |
| mnaser | let me include a screenshot | 19:19 |
| tristanC | corvus: i'm not sure if you are familiar with this web site: https://matrix.yaml.io/, but it seems like that anchor syntax is not fully supported: https://matrix.yaml.io/details/ZH7C.html | 19:19 |
| mnaser | pabelanger this is what our branch protection rules look like with it https://usercontent.irccloud-cdn.com/file/1hpeRoM5/image.png | 19:20 |
| pabelanger | mnaser: ah, thanks! | 19:20 |
| pabelanger | mnaser: why no gate check? | 19:20 |
| corvus | tristanC: do i understand that you're saying that "perl-tiny.json" doesn't implement it? | 19:20 |
| mnaser | pabelanger: i should probably add it there, probably in my debugging, because the gate was a tricky one get to enqueue | 19:21 |
| pabelanger | mnaser: kk | 19:21 |
| mnaser | let me add the gate pipeline | 19:22 |
| mnaser | pabelanger: oh the pipeline is very simple, its the github branch protection that enforces it doesnt merge/queue before check and lgtm are passing | 19:22 |
| tristanC | corvus: there are other test cases too, what i'm saying is that such features may break non libyaml based parser, so if an anchor is not needed, then it sounds better to not use it | 19:22 |
| pabelanger | mnaser: I also wonder if you could add /lgtm trigger in zuul pipeline, to then enqueue job into lgtm pipeline | 19:23 |
| tristanC | corvus: it's the first time i ever see an anchor starting on a new line for a string | 19:23 |
| corvus | tristanC: i guess i don't understand that page then, because my interpretation is that the only one that doesn't support it is yaml::tiny for perl | 19:23 |
| pabelanger | mnaser: but again, thanks! I'll hack on this for sure to test | 19:23 |
| corvus | tristanC: i understand the sentiment, however as a rule we use anchors fairly extensively in the zuul project's .zuul.yaml files, so i don't think we should adopt that practice generally. i agree that it's not necessary, but i don't think it's worth fretting over since, after all, it made copy-pasta easier | 19:25 |
| ianw | corvus: web things; were you ok with the final state reached in https://review.opendev.org/#/c/763126 for the build/buildset pages -- that stacks on felixedel's original to make both the name and status clickable, and removes the search icon | 19:25 |
| pabelanger | mnaser: oh, I think I am understanding. There is no lgtm app on github you install, you either use existing zuul github app (or create new one) and the job in the lgtm pipeline does the magic | 19:27 |
| corvus | ianw: yes. left a comment on there. do you think we can squash those? | 19:29 |
| ianw | corvus/felixedel: i am happy for that stack of 3 (base work, clickable links, remove icon) to be squashed if felixedel is; the basic stuff is all his work | 19:30 |
| tristanC | corvus: i wonder if that extensive usage of anchor and merge in zuul.yaml could be a motivation for a new zuul `vars` object to be used for sharing variables between jobs | 19:32 |
| corvus | ianw: sorry my comment was on 762588 | 19:32 |
| corvus | tristanC: could be? but there's a reason we're using anchors here in that the only duplication is deep inside a data structure which is otherwise not duplicated | 19:33 |
| corvus | so i mean, there's a real danger it ends up looking like yaml anchors | 19:34 |
| tristanC | corvus: i meant something that could work accross file and projects | 19:34 |
| openstackgerrit | Merged zuul/zuul-registry master: Add upload-docker-image release job https://review.opendev.org/763419 | 19:35 |
| ianw | corvus: i can try an underline in a follow-on and we can compare. i did originally and i thought it didn't look very good | 19:35 |
| corvus | ianw: sgtm. i thought there was a chance of that (thus the +2 with comment) | 19:35 |
| ianw | corvus: while talking web stuff, another that hasn't had much comment is a title-bar for the tenants page @ https://review.opendev.org/#/c/756941/ | 19:36 |
| tristanC | ianw: there is also https://review.opendev.org/759969 to fix filter accross tabs (reported by dmsimard) | 19:37 |
| ianw | tristanC: oh sorry i thought we fixed that. maybe i just confirmed the problem, i definitely remember playing with that. will review | 19:38 |
| corvus | we *just* added the localstorage (cc zbr) | 19:38 |
| corvus | i guess the difference with the old cookie one is that the cookie only mattered when you loaded the page, and now this updates in real time across tabs? | 19:39 |
| ianw | somebody posted like a gif or a recording of it happening, was that for this change? | 19:41 |
| corvus | ianw: i don't think that's necessary, you can confirm by visiting zuul.opendev.org | 19:42 |
| ianw | http://eavesdrop.openstack.org/irclogs/%23zuul/%23zuul.2020-10-27.log.html#t2020-10-27T20:46:03 | 19:43 |
| ianw | this is the conversation i'm thinking of | 19:43 |
| ianw | and https://imgur.com/IjTf9om is the gif i'm thinking of | 19:43 |
| corvus | ianw: thanks for the refresher, that saved a lot of typing :) | 19:45 |
| corvus | tristanC: lgtm | 19:45 |
| corvus | pabelanger: commit ea4b756f70a41e51c7dc0aed7f17df31da461c91 (HEAD -> master, tag: 1.0.0, origin/master, origin/HEAD, refs/changes/19/763419/2) | 19:46 |
| corvus | pabelanger: that look right to you? | 19:46 |
| pabelanger | mnaser: okay, so /lgtm is +1 in gerrit, and /approve is +2 right? | 19:46 |
| pabelanger | corvus: zuul-registry, lgtm | 19:47 |
| corvus | pabelanger: and for nodepool: commit afe86884d291cf873b855a76e84ad209e0560ab2 (HEAD -> master, tag: 3.14.0, origin/master, refs/changes/36/762836/1) | 19:48 |
| corvus | zuul-maint: ^ anyone else want to double check those release shas for zuul-registry and nodepool? | 19:48 |
| pabelanger | corvus: lgtm | 19:49 |
| mnaser | pabelanger: /lgtm is like +2 in approve, /approve is like workflow | 19:51 |
| pabelanger | ah right | 19:51 |
| pabelanger | thanks | 19:51 |
| openstackgerrit | Simon Westphahl proposed zuul/zuul master: Prune git tags on fetch https://review.opendev.org/761756 | 19:53 |
| swest | corvus: pabelanger: ^ restored it. not sure if we need to add a fallback for older git versions. | 19:55 |
| pabelanger | mnaser: for github app, are you sharing you zuul.conf github app key, or did you create a new one? | 19:56 |
| mnaser | pabelanger: yes that is correct wrt lgtm :) | 19:57 |
| mnaser | pabelanger: and same app in our case | 19:57 |
| pabelanger | part of me thinks a 2nd key is the way to go, as not to expose it on node | 19:57 |
| pabelanger | or some how expose the ability for a job to zuul_return zuul comment on PR / change | 19:58 |
| *** hashar has quit IRC | 19:58 | |
| openstackgerrit | Paul Belanger proposed zuul/zuul master: WIP: Support ansible 2.10 https://review.opendev.org/757452 | 20:07 |
| mnaser | pabelanger: btw the job is actually a 0 node job :) | 20:10 |
| mnaser | so its quite fast | 20:10 |
| pabelanger | mnaser: yah, we do that for unlabel-on-push jobs | 20:11 |
| corvus | pabelanger: if it's zero-node, it may be relatively safe to use the same secret. i mean, a different one may be better. just pointing out that writing a secret out to the executor isn't the same exposure potential as to a node. | 20:12 |
| pabelanger | corvus: yup, agree | 20:13 |
| pabelanger | I'm not sure we have exposed control plane secrets into our zuul yet | 20:14 |
| *** wuchunyang has joined #zuul | 20:21 | |
| *** wuchunyang has quit IRC | 20:26 | |
| mnaser | corvus, pabelanger: having said that, i think with github you can use different certs for the app | 20:38 |
| mnaser | so you can revoke specific ones | 20:38 |
| mnaser | yes, you can have multiple secrets for teh same app | 20:41 |
| mnaser | also obviously we have lgtm inside a trusted repo too | 20:41 |
| corvus | mnaser: can you expand on that last comment? | 20:42 |
| corvus | (i don't fully understand what's in a trusted repo) | 20:43 |
| mnaser | corvus: sorry, config-project is the right term :) | 20:43 |
| mnaser | my brain thinks untrusted project => opposite is trusted :p | 20:43 |
| corvus | well, that part i understood :) | 20:43 |
| corvus | i meant i don't understand what specifically is in that repo | 20:43 |
| mnaser | ah, yes, the pipeline / job / secret / etc | 20:44 |
| mnaser | so we simply 'attach' that extra repository into config-projects and it starts working | 20:44 |
| corvus | i think the only thing you need in a config project (which runs in the "trusted" execution context ;) is the job definition+playbook. (ie, not the role, that can be in an untrusted project like zuul-jobs) | 20:44 |
| corvus | mnaser: oh i see, you bundle up the those 4 things as a unit in a purpose-build git repo which you attach to whatever tenants want that feature | 20:45 |
| corvus | mnaser: ++ | 20:45 |
| fungi | that's an interesting approach to modularity | 20:45 |
| mnaser | corvus: right, yes, so i don't have to worry about any weird things being introduced into this equation | 20:46 |
| corvus | it's a good approach | 20:47 |
| mnaser | ideally a nicer thing to do would be | 20:47 |
| mnaser | allow us to also use zuul_return in a way to post the comment (but we still need api keys to get the comments for private repos) | 20:48 |
| mnaser | if we can get all the comments fed in (which might start to be asking for a lot) then this can be entirely native without creds | 20:49 |
| corvus | mnaser: iiuc the bit missing from zuul_return is ability to leave a top-level pr comment (only line comments are currently supported)? | 20:51 |
| mnaser | i can honestly imagine zuul_return leaving a commenet can introduce a lot of really interesting use cases | 20:52 |
| mnaser | like for example things like terraform users who want a plan left as a comment, and if its added in a generic way, it'd work regardless if you use gerrit/github/gitlab/etc | 20:52 |
| corvus | mnaser: ^ can you confirm my understanding of what you're suggesting? | 20:52 |
| mnaser | corvus: well that would help with _one_ part of it, but not all of it (im thinking a way to actually parse all the comments of a change) | 20:53 |
| mnaser | so i can for example loop over zuul.change.comments | 20:53 |
| corvus | mnaser: that latter as a pipeline trigger or requirement? | 20:53 |
| mnaser | corvus: as a job variable | 20:53 |
| corvus | mnaser: gotcha | 20:54 |
| corvus | mnaser: both of those seem reasonable and good ideas at first glance | 20:55 |
| mnaser | corvus: yeah, and with both of these, i think it would entirely eliminate the need for app creds to be present in the job | 20:55 |
| openstackgerrit | Slawek Kaplonski proposed zuul/zuul-jobs master: [multi-node-bridge] Add script to configure connectivity https://review.opendev.org/762650 | 21:01 |
| *** nils has quit IRC | 21:23 | |
| openstackgerrit | Ian Wienand proposed zuul/zuul master: web: Add optional link prop to title-with-icon and results https://review.opendev.org/763471 | 22:04 |
| ianw | corvus / felixedel: ^ that makes things underlined, and is probably more react-y overall | 22:05 |
| corvus | ianw: thx | 22:13 |
| corvus | pabelanger: https://pypi.org/project/zuul-registry/1.0.0/ exists | 22:26 |
| corvus | pabelanger: https://hub.docker.com/r/zuul/zuul-registry/tags also looks correct | 22:27 |
| corvus | pabelanger: you got it in one! awesome! thank you :) | 22:27 |
| corvus | i think that's a new record score for "release job golf" :) | 22:28 |
| corvus | i just pushed nodepool | 22:29 |
| *** armstrongs has joined #zuul | 22:39 | |
| *** armstrongs has quit IRC | 22:45 | |
| *** Goneri has quit IRC | 23:10 | |
| *** hamalq has quit IRC | 23:53 | |
| *** hamalq has joined #zuul | 23:54 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!
