| *** rfolco has quit IRC | 00:51 | |
| openstackgerrit | Pierre-Louis Bonicoli proposed zuul/zuul-jobs master: Avoid to use 'length' filter with null value https://review.opendev.org/742316 | 01:28 |
|---|---|---|
| openstackgerrit | Pierre-Louis Bonicoli proposed zuul/zuul-jobs master: Avoid to use 'length' filter with null value https://review.opendev.org/742316 | 01:32 |
| *** wuchunyang has joined #zuul | 02:16 | |
| *** pots has quit IRC | 02:26 | |
| *** pots has joined #zuul | 02:27 | |
| *** wuchunyang has quit IRC | 02:36 | |
| *** bhavikdbavishi has joined #zuul | 04:01 | |
| *** bhavikdbavishi1 has joined #zuul | 04:04 | |
| *** bhavikdbavishi has quit IRC | 04:05 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 04:05 | |
| *** smyers has quit IRC | 04:16 | |
| *** evrardjp has quit IRC | 04:33 | |
| *** raukadah is now known as chkumar|rover | 04:33 | |
| *** evrardjp has joined #zuul | 04:33 | |
| *** smyers has joined #zuul | 04:33 | |
| *** saneax has joined #zuul | 04:38 | |
| *** Goneri has quit IRC | 04:44 | |
| swest | corvus: my comments were towards the modified method signatures in test_github_driver.py and not in anything unrelated to that change. I still stand by my statement that mutable defaults are never a good idea, even if currently not modified or it's 'just test code'. | 05:40 |
| *** yolanda has quit IRC | 05:56 | |
| *** yolanda has joined #zuul | 05:58 | |
| *** bhavikdbavishi has quit IRC | 06:47 | |
| *** jcapitao has joined #zuul | 06:58 | |
| *** wuchunyang has joined #zuul | 07:19 | |
| *** frickler_pto is now known as frickler | 07:26 | |
| *** wuchunyang has quit IRC | 07:34 | |
| *** tosky has joined #zuul | 07:35 | |
| *** hashar has joined #zuul | 07:38 | |
| *** bhavikdbavishi has joined #zuul | 07:40 | |
| *** sgw1 has quit IRC | 07:40 | |
| openstackgerrit | Ian Wienand proposed zuul/zuul master: github status api docs : update https://review.opendev.org/744406 | 07:47 |
| *** holser has joined #zuul | 07:56 | |
| *** jpena|off is now known as jpena | 07:56 | |
| *** sshnaidm|afk is now known as sshnaidm | 07:58 | |
| *** ttx has quit IRC | 08:00 | |
| *** ttx has joined #zuul | 08:01 | |
| *** nils has joined #zuul | 08:07 | |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Scheduler's pause/resume functionality https://review.opendev.org/709735 | 08:19 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Separate connection registries in tests https://review.opendev.org/712958 | 08:19 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Prepare Zookeeper for scale-out scheduler https://review.opendev.org/717269 | 08:19 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Mandatory Zookeeper connection for ZuulWeb in tests https://review.opendev.org/721254 | 08:19 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Driver event ingestion https://review.opendev.org/717299 | 08:19 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: WIP: Store unparsed branch config in Zookeeper https://review.opendev.org/705716 | 08:19 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Connect merger to Zookeeper https://review.opendev.org/716221 | 08:27 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Connect fingergw to Zookeeper https://review.opendev.org/716875 | 08:27 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: Connect executor to Zookeeper https://review.opendev.org/716262 | 08:27 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: WIP: Switch to using zookeeper instead of gearman for jobs (keep gearman for mergers) https://review.opendev.org/744416 | 08:27 |
| openstackgerrit | Jan Kubovy proposed zuul/zuul master: WIP: Switch to using zookeeper instead of gearman for jobs (keep gearman for mergers) https://review.opendev.org/744416 | 08:28 |
| *** bolg has joined #zuul | 08:36 | |
| *** jcap has joined #zuul | 09:16 | |
| *** jcap has quit IRC | 09:16 | |
| *** jcapitao has quit IRC | 09:17 | |
| *** jcapitao has joined #zuul | 09:17 | |
| *** hashar has quit IRC | 09:28 | |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab: support the approval requirement https://review.opendev.org/741637 | 09:54 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab: support the labeled event https://review.opendev.org/741667 | 09:54 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab: support the labels requirement https://review.opendev.org/741893 | 09:54 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab: support the merge reporter https://review.opendev.org/741931 | 09:54 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab: remove default mutables https://review.opendev.org/741938 | 09:54 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab: support the MR merged event https://review.opendev.org/742107 | 09:54 |
| *** wuchunyang has joined #zuul | 10:29 | |
| *** wuchunyang has quit IRC | 10:33 | |
| *** chkumar|rover is now known as chandankumar | 11:12 | |
| *** chandankumar is now known as chkumar|rover | 11:13 | |
| *** jpena is now known as jpena|lunch | 11:21 | |
| *** rfolco has joined #zuul | 11:54 | |
| *** jcapitao is now known as jcapitao_lunch | 12:01 | |
| *** rlandy has joined #zuul | 12:02 | |
| *** _erlon_ has joined #zuul | 12:19 | |
| *** jpena|lunch is now known as jpena | 12:23 | |
| cloudnull | mornings | 12:33 |
| tristanC | cloudnull: good morning o/ | 12:58 |
| cloudnull | p/ | 12:58 |
| fungi | swest: if mutable defaults are never a good idea, why did python3 keep them ever after decades of prior art to inform the choice? (it made a number of other backward-incompatible changes, so certainly could have altered that detail as well) | 13:02 |
| tristanC | swest: i'm also curious what type of issue can you have when mutating a default? Do the modified value persist accross call? | 13:07 |
| *** bhavikdbavishi has quit IRC | 13:07 | |
| fungi | tristanC: yes, it's effectively global state, so subsequent calls continue to use the updated mutable value | 13:08 |
| fungi | consider it an implicit pass by reference | 13:09 |
| fungi | i think that's the main objection some folks have, it's at odds with the python principle of "better explicit than implicit" | 13:10 |
| tristanC | fungi: makes me wonder in which situation would that be desirable? | 13:12 |
| fungi | which is why you'll see functions frequently define a parameter like def foo(..., bar=None, ...): and then reassign within the function like if bar == None: bar = list() | 13:12 |
| fungi | tristanC: well, for one, if you know the function won't be called multiple times then you don't have to worry about keeping it reentrant and can dispense with the extra reassignment | 13:13 |
| fungi | but also if the program is designed to assume global state for something you want instantiated on the first call into that function, it can save you additional reference passing in long chains of nested function calls without separate global defines | 13:14 |
| swest | fungi: I think there are certain situations were mutable defaults make sense, that's probably why Python 3 allows them. But IMHO there needs to be a strong reason for that. I think "it needs a few more lines of code" doesn't fall into that category. | 13:14 |
| fungi | swest: yeah, i agree it's preferable to at least assert that it's being done intentionally (and why), since it's all too easy to do accidentally and then get surprised by later when your "default" values don't seem very default on subsequent calls into the same function | 13:16 |
| swest | fungi: I don't see any particular reason for that in Zuul, hence my review to not introduce more of that. And btw. I did not sugggest changing any of that in the existing test code. | 13:17 |
| fungi | but also "it was done this way previously and so i'm keeping consistency with preexisting code" is a good reason, at least until a refactor | 13:17 |
| swest | fungi: repeating a bad pattern just for consistencies sake sounds like a bad argument ;) | 13:18 |
| fungi | no worse of an argument than not fixing all existing occurrences | 13:19 |
| swest | I'd be all for that, but apparently that doesn't seem to be wanted?! | 13:19 |
| fungi | having a random mix where some are one way and some are the other can make the code harder to maintain | 13:19 |
| tristanC | fungi: thank you for the examples. Though it seems like they contradict with the zen of python | 13:20 |
| *** jcapitao_lunch is now known as jcapitao | 13:20 | |
| fungi | tristanC: i agree with you, that's the case for the most part | 13:20 |
| fungi | (but also, not all python is especially zen) | 13:20 |
| tristanC | and thank you for the discussion, i didn't knew python behaved like that :-) | 13:21 |
| fungi | tristanC: many folks don't, which is part of why they can be easily surprised when trying to troubleshoot the weird and unexpected behaviors resulting from unintended mutable default reuse | 13:21 |
| fungi | there are of course exceptions, but again they *could* be implemented in more explicit (and arguably inelegant) ways: http://effbot.org/zone/default-values.htm#valid-uses-for-mutable-defaults | 13:28 |
| *** jhesketh has quit IRC | 14:14 | |
| *** jhesketh has joined #zuul | 14:16 | |
| *** sgw1 has joined #zuul | 14:28 | |
| *** masterpe has joined #zuul | 14:35 | |
| *** masterpe has quit IRC | 14:47 | |
| *** bhavikdbavishi has joined #zuul | 15:00 | |
| *** jpena is now known as jpena|off | 15:52 | |
| corvus | tobiash, swest: we just ran into the nodepool json empty string parse error (related to https://review.opendev.org/738013 ) but with a build rather than an upload | 16:15 |
| *** hamalq has joined #zuul | 16:16 | |
| *** chkumar|rover is now known as raukadah | 16:18 | |
| *** nils has quit IRC | 16:22 | |
| *** jcapitao has quit IRC | 16:22 | |
| *** tosky has quit IRC | 16:45 | |
| clarkb | corvus: looking at the yappi situation the python-builder assemble script should install it I think | 16:47 |
| clarkb | corvus: it attempts to detect any extras in the package and installs them (and yappi is listed as a nodepool extras) | 16:47 |
| clarkb | and my laptop just crashedpulling the image. Its almost like any cpu governor change trips it | 16:51 |
| *** sshnaidm has quit IRC | 16:55 | |
| clarkb | huh left it alone and eventually it recovered, so weird | 16:56 |
| clarkb | ok I've confirmed there seems to be no yappi and objgraph in the image | 16:57 |
| clarkb | I think its a path mismatch | 16:59 |
| *** bhavikdbavishi has quit IRC | 17:01 | |
| swest | corvus: interesting. I think in our case it was a restart/crash of a builder while multiple builder were trying to cleanup the same upload | 17:02 |
| swest | but I did not have enough logs to confirm that | 17:03 |
| *** jpena|off is now known as jpena | 17:04 | |
| *** jpena is now known as jpena|off | 17:17 | |
| *** saneax has quit IRC | 17:20 | |
| clarkb | ok I've figured out what the extras thing is trying to do | 17:21 |
| clarkb | and running the steps manually locally it seems to probably be ok. That doesn't explain why we don't get yappi and objgraph in the final image though | 17:22 |
| clarkb | oh ha I think I see the bug | 17:25 |
| clarkb | remote: https://review.opendev.org/744531 Use pip install -r not -f to install extras | 17:29 |
| clarkb | I think ^ will fix it for nodepool and zuul image builds | 17:29 |
| clarkb | however zuul's image build isn't installing extras yet | 17:30 |
| clarkb | that has me wondering how the zuul scheduler is getting pymysql | 17:31 |
| fungi | huh, yeah --find-links /some/path is supposed to "look for archives in the directory listing" | 17:33 |
| clarkb | oh zuul fixes this by working around the bug | 17:35 |
| clarkb | https://opendev.org/zuul/zuul/src/branch/master/Dockerfile#L55 that is why zuul works | 17:35 |
| clarkb | so ya once the fix lands we can update the dockerfile for zuul to drop the workaround and nodepool should start to work | 17:35 |
| openstackgerrit | Clark Boylan proposed zuul/zuul master: Simplify dockerfile https://review.opendev.org/744533 | 17:39 |
| clarkb | thats the small zuul simplification based on the python-builder fix | 17:40 |
| *** armstrongs has joined #zuul | 17:51 | |
| *** tosky has joined #zuul | 18:08 | |
| *** armstrongs has quit IRC | 18:09 | |
| *** sshnaidm has joined #zuul | 18:14 | |
| corvus | mordred: i'm confused about node_version. what in 739047 actually uses it? | 18:29 |
| corvus | is it build-javascript-deployment? | 18:30 |
| corvus | mordred: that should have the variable set already | 18:31 |
| *** hashar has joined #zuul | 18:31 | |
| *** hashar has quit IRC | 20:23 | |
| *** sgw1 has quit IRC | 20:47 | |
| *** sgw1 has joined #zuul | 20:49 | |
| tristanC | is there a way for a child job to erase attribute inherited from a parent job? For example to provide it's own set of required-project | 20:50 |
| tristanC | at first glance that doesn't seem possible, but perhaps there could be a semantic to indicate an attribute shouldn't be extended from parent definition? | 20:57 |
| corvus | tristanC: no; the usual solution is to add in an extra entry to the hierarchy | 21:49 |
| mordred | corvus: ah - yeah - there's an issue there - it's not needed on publish-javascript-to-netlify | 21:55 |
| mordred | but we do need to set it on the test job because publish-javascript-to-netlify gets it from the js-build base job | 21:56 |
| openstackgerrit | Monty Taylor proposed zuul/zuul-jobs master: Add a job for publishing a site to netlify https://review.opendev.org/739047 | 21:56 |
| corvus | mordred: ah that makes more sense :) | 21:56 |
| tristanC | corvus: in our situation we would like to discard the parent attribute. It seems like one way to solve this is using tenant config shadow and provide our own copy of the job | 21:57 |
| tristanC | I was wondering if it is a good idea to support this in the job definition, for example using an explicit null value to indicate an attribute should not be inherited | 21:59 |
| corvus | tristanC: i encourage you to consider another approach. think of this as indicating that the modeling of the job is not currently correct. by adding another layer to the hierarchy, you can add in what's needed in a more fine-grained manner. | 21:59 |
| corvus | tristanC: er, wait, are you not in control of the parent job? | 21:59 |
| tristanC | corvus: so the situation is another zuul using the tripleo-ci-base job provided by opendev.org/openstack/tripleo-ci/ | 22:00 |
| corvus | tristanC: if it's the case that you don't control the parent job, then yes -- first i'd suggest maybe talking to the folks who do control it to see about adding in another layer (eg, devstack-minimal -> devstack). or failing that, then, yes, shadowing would be appropriate (hopefully while being able to re-use roles) | 22:01 |
| corvus | tristanC: and the issue is that the required-projects over-specify the names? (eg, they specify "opendev.org/openstack/tripleo-quickstart" instead of "openstack/tripleo-quickstart" ?) | 22:03 |
| tristanC | corvus: the trick is that tripleo-ci-base job is used indirectly, i think the parenting start from one of its child | 22:03 |
| corvus | tristanC: why is required-projects wrong for this case? | 22:04 |
| corvus | zuul-maint: i think it's time to send this email re v4/v5; how does this look? https://etherpad.opendev.org/p/6Q5yrO5mVPUKIRMDW3tm | 22:05 |
| fungi | corvus: lgtm, thanks! | 22:07 |
| clarkb | corvus: maybe link to docs for eahc of the 3 required items? | 22:08 |
| corvus | the relnotes do that, but i'll copy it in | 22:10 |
| tristanC | corvus: it seems like there is a conflict or something, but a similar issue happened with irrelevant-files, where basically a job would like to override an attribute (or change the parent) of a parent job | 22:10 |
| corvus | tristanC: right, it's an issue, it's just the solution varies depending on the circumstances; i'd like to understand this circumstance more | 22:11 |
| corvus | (it's unclear to me why a project would be listed in required-projects if it's not required) | 22:11 |
| openstackgerrit | Clark Boylan proposed zuul/zuul-jobs master: Fix partial subunit stream logging https://review.opendev.org/744565 | 22:12 |
| ianw | this question re status updates that zuul puts into github -- https://github.com/pyca/cryptography/pull/5341#issuecomment-667574606 | 22:14 |
| ianw | i've tried to bootstrap myself on what's happening ... so we're not putting individual job results into githubs results for ... many reasons ... but basically it doesn't work with pipelines | 22:15 |
| ianw | does anyone have any other thoughts to respond with? | 22:15 |
| tristanC | corvus: perhaps the job implementation uses a firstfound lookup that is affected by what is present on the test instance... | 22:16 |
| corvus | ianw: iirc, opendev isn't using checks api; we could switch to that | 22:16 |
| corvus | tristanC: i don't understand | 22:16 |
| clarkb | corvus: is that a pipeline config or something we hvae to change in the zuul ini config? | 22:17 |
| corvus | tristanC: i'd like to help, but in order to do so, i think i need a clear statement of the underlying problem | 22:17 |
| corvus | clarkb: pipeline i think | 22:17 |
| tristanC | well I guess enabling a different parent hierarchy is not a good idea and using an appropriate job hierarchy is a better solution | 22:17 |
| ianw | corvus / clarkb: yes, i was bootstrapping myself on the checks api which is why i came up with https://review.opendev.org/#/c/744406/1/doc/source/discussion/github-checks-api.rst | 22:19 |
| corvus | ianw: let me check one other thing first | 22:19 |
| ianw | but it's not clear to me if this will provide more info in the results overview | 22:19 |
| ianw | corvus: no rush :) | 22:19 |
| ianw | the other thing is how to setup checks on pushes to master | 22:20 |
| ianw | ... i had a bit of a blind spot to this because post-hoc testing showing your push was broken is not a thing i think about, thankfully :) | 22:22 |
| clarkb | ianw: for gerrit you can listen to update to refs/heads/* | 22:23 |
| clarkb | I imagine we can do similar with github | 22:24 |
| ianw | yeah i think it has a push event ... i don't want it to go mad though as i don't have any experience with it | 22:24 |
| corvus | ianw: i expect the buildset page to be the status url on report | 22:24 |
| corvus | ianw: however, it looks like it's just reported the top level dashboard url, right? | 22:25 |
| ianw | corvus: i think that might be sufficient. yeah, it goes to openstack.org status or something totally unrelated | 22:25 |
| corvus | ianw: then i think it's worth looking into why that isn't being set as expected | 22:25 |
| ianw | ok, https://zuul.openstack.org | 22:25 |
| clarkb | zuulians https://review.opendev.org/#/c/744107/1 may be a good one to approve to rebuild nodepool images with the python extras packages | 22:26 |
| clarkb | I've rechecked https://review.opendev.org/#/c/744533/1 too now that the depends on has merged to rebuild and test with the proper parent image state | 22:26 |
| tristanC | corvus: not sure if that's it, but for example here the job is doing things based on what is available on the test instance: https://opendev.org/openstack/tripleo-ci/src/branch/master/roles/run-test/templates/toci_gate_test.sh.j2#L143 | 22:27 |
| tristanC | corvus: (the v4/v5 announces looks great to me) | 22:28 |
| corvus | tristanC: my first thought is still: if it's not actually required, maybe it shouldn't be in required-projects, at least at that level. | 22:29 |
| corvus | ianw: i'm *especially* puzzeled why it would use that url | 22:29 |
| tristanC | corvus: thank you for offering to help, but i think you are right and the current modeling of the job is not correct | 22:30 |
| corvus | ianw: oh i think i see | 22:30 |
| ianw | corvus: perhaps the detail link is the final one that ends up in the status ... https://review.opendev.org/#/c/744384/1/zuul.d/pipelines.yaml would i think override this | 22:32 |
| ianw | hrm it looks like it calls formStatusUrl() https://opendev.org/zuul/zuul/src/branch/master/zuul/model.py#L2781 | 22:34 |
| ianw | https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/github/githubreporter.py#L232 | 22:34 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: github: use the same status url for commit status as checks https://review.opendev.org/744570 | 22:34 |
| corvus | ianw: ^ that match your understanding? | 22:35 |
| ianw | corvus: haha yes you beat me to it :) | 22:35 |
| corvus | ianw: having said that, perhaps opendev should switch to using checks api as well | 22:36 |
| ianw | aiui, moving to the checks api doesn't really do that much different? | 22:37 |
| ianw | it's still the overall pipeline status that is reported | 22:37 |
| corvus | ianw: not at this point; mostly it would change how it's represented in the ui, and also allow file comments | 22:37 |
| ianw | right,that's what i was learning about yesterday, how the check run object seems at first glance to be a really good analogue for a pipeline, but isn't :) | 22:38 |
| corvus | ianw: biggest thing is we'd show up in the "checks" tab | 22:38 |
| corvus | anyway, nbd | 22:39 |
| ianw | i don't mind trying, i guess i can just modify it in the pyca/project-config and recheck on our test change? | 22:40 |
| corvus | tobiash: https://review.opendev.org/744406 | 22:42 |
| corvus | ianw: i'm 95% certain my change will fail tests (probably need to update test fixture data to match); switching to checks will probably resolve the issue sooner | 22:43 |
| ianw | corvus: https://review.opendev.org/744573 Switch to checks API look about right? | 22:48 |
| openstackgerrit | Clark Boylan proposed zuul/zuul-jobs master: Fix partial subunit stream logging https://review.opendev.org/744565 | 22:52 |
| *** tosky has quit IRC | 23:11 | |
| openstackgerrit | Ian Wienand proposed zuul/zuul master: github docs: clarify situation with checks/status API https://review.opendev.org/744577 | 23:13 |
| ianw | corvus: ^ reflecting what i think i've learnt :) | 23:14 |
| *** hamalq has quit IRC | 23:50 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!