| *** rlandy is now known as rlandy|out | 00:08 | |
| *** ysandeep|out is now known as ysandeep | 01:19 | |
| opendevreview | Ian Wienand proposed openstack/project-config master: proposal updates: fix typo in release match https://review.opendev.org/c/openstack/project-config/+/851156 | 04:50 |
|---|---|---|
| opendevreview | Merged openstack/project-config master: proposal updates: fix typo in release match https://review.opendev.org/c/openstack/project-config/+/851156 | 05:11 |
| *** ysandeep is now known as ysandeep|afk | 06:08 | |
| *** jpena|off is now known as jpena | 07:35 | |
| *** ysandeep|afk is now known as ysandeep|lunch | 08:00 | |
| opendevreview | Ke Niu proposed openstack/reviewstats master: remove unicode from code https://review.opendev.org/c/openstack/reviewstats/+/851171 | 08:51 |
| *** ysandeep|lunch is now known as ysandeep | 10:01 | |
| *** rlandy|out is now known as rlandy | 10:34 | |
| *** dviroel|out is now known as dviroel | 11:35 | |
| *** ysandeep is now known as ysandeep|break | 12:19 | |
| *** ysandeep|break is now known as ysandeep | 12:58 | |
| *** rlandy is now known as rlandy|biab | 13:17 | |
| *** rlandy|biab is now known as rlandy | 13:33 | |
| *** akekane_ is now known as abhishekk | 14:12 | |
| *** dasm|off is now known as dasm | 14:42 | |
| *** ysandeep is now known as ysandeep|out | 14:54 | |
| *** dviroel is now known as dviroel|lunch | 14:57 | |
| elodilles | ianw: we started to see release job failures, see my comment here: https://review.opendev.org/c/openstack/project-config/+/850962/5#message-73ebe871b437f4e388bc693eb8892f36d337a0aa | 15:21 |
| elodilles | ianw: is it possible that this has already fixed the issue? https://review.opendev.org/c/openstack/project-config/+/851156 | 15:21 |
| elodilles | ianw: and if it is fixed by that patch, when can we be sure that it is synchronized to zuul and we can approve release job again? | 15:39 |
| clarkb | elodilles: once a change merges to project-config jobs starting after that merge point should see the new content | 15:41 |
| clarkb | there isn't a separate synchronization step | 15:42 |
| elodilles | clarkb: oh, i see. i just thought there's some delay as we started to see the failures half day later, while there were merged patches in the meantime. | 15:50 |
| elodilles | in that case that patch did not fix our issue :/ | 15:50 |
| *** dviroel|lunch is now known as dviroel | 15:52 | |
| tinwood | qq: if we've just set up a new project, but there's never been a commit via gerrit, would it be true that the github mirror would be empty? Everything else looks fine in the config so that's the only thing I can think of? https://review.opendev.org/q/project:openstack/charm-cinder-dell-emc-powerstore -- thanks! | 15:57 |
| clarkb | elodilles: https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/ensure-pip/tasks/main.yaml#L77-L80 is the task that is failing. I don't understand why. That was called by https://opendev.org/openstack/project-config/src/branch/master/playbooks/proposal/pre.yaml#L79-L84 using the focal path whcih does not set _venv_command. Maybe the default(omit) is causing problems? | 16:00 |
| clarkb | tinwood: replication to github happens via zuul jobs and at first glance I see none configured for that project | 16:00 |
| jrosser | theres almost no case that omit should be used other than on a module parameter | 16:01 |
| clarkb | jrosser: I think the idea is we want the role default to take place there if _venv_command isn't set | 16:02 |
| clarkb | I guess default(omit) isn't the way to express that | 16:02 |
| clarkb | maybe we need to calls to ensure-tox guarded by a when for when _venv_command is set and another for when it isn't set | 16:02 |
| jrosser | no, it says to an ansible module "nothing to see here" when you specify a parameter that sometimes you want to be absent | 16:02 |
| jrosser | used in a var it turns into something like omit_$huge_massive_string_here | 16:03 |
| clarkb | I see. Is there a better way to express "use the default if venv_command isn't set" than calling ensure-tox two different ways with when conditions? | 16:04 |
| tinwood | clarkb, thanks; let me go check; I can see it has no .zuul.conf, which we'll need to add. | 16:05 |
| clarkb | tinwood: the github sync job may need to go into openstack/project-config/zuul.d/projects.yaml to have access to the necessary secrets | 16:05 |
| fungi | i think it's usually done via a template for official openstack projects? | 16:06 |
| tinwood | clarkb, yes, it's normally automatic for all the charm-* projects. however, there's no mention of it in project-config:zuul.d/projects.yaml -- I'll have a stare and see if I need to add something. | 16:10 |
| opendevreview | Clark Boylan proposed openstack/project-config master: Set ensure_pip_virtualenv_command in Focal proposal jobs https://review.opendev.org/c/openstack/project-config/+/851239 | 16:10 |
| clarkb | elodilles: jrosser ^ something like that maybe | 16:10 |
| tinwood | clarkb, is "official-openstack-repo-jobs" the template that does the sync? I see all our other charm projects use that, which is probably what is missing. | 16:11 |
| clarkb | tinwood: fungi: the template appears to be official-openstack-repo-jobs | 16:12 |
| tinwood | clarkb, thanks; I'll raise a review to fix. | 16:12 |
| clarkb | if you look in that file I mentioned all the cahrm repos have an entry for that template but I don't see this particular repo. Adding that should be what is necessary here | 16:12 |
| jrosser | clarkb: the only other thing i can think of is to set_fact ensure_pip_virtualenv_command rather than pass it as a var, but it's arguable thats much less obvious what the intent is | 16:13 |
| clarkb | ya this is quite verbose but at least it should make the intent clear | 16:16 |
| jrosser | i think you'd be able to get rid of a lot of conditional code by using vars/<operating-system>.yml type approaches | 16:17 |
| jrosser | and including the OS specific vars at the top of the role, which may, or may not override included role defaults as needed | 16:18 |
| jrosser | then you make it kind of data driven rather than having to express all the logic in the code | 16:18 |
| clarkb | ya that might be another appraoch. Can you have a vars/ dir adjacent to playbooks or only roles? | 16:19 |
| jrosser | lots of places, but beware the vars precedence | 16:20 |
| jrosser | we do this just as boilerplate in all roles https://github.com/openstack/openstack-ansible-os_keystone/blob/master/tasks/main.yml#L34-L48 | 16:21 |
| clarkb | ya we've got some of that in our system management roles. But it happens less in the zuul job stuff. I think beacuse it isn't super common for a job to run across a ton of platforms. Though maybe that happens in devstack and similar locations like osa jobs too | 16:22 |
| *** jpena is now known as jpena|off | 16:32 | |
| opendevreview | Alex Kavanagh proposed openstack/project-config master: Complete charm-cinder-dell-emc-powerstore config https://review.opendev.org/c/openstack/project-config/+/851242 | 16:40 |
| tinwood | clarkb, fungi - thanks for the merge. | 16:51 |
| opendevreview | Merged openstack/project-config master: Complete charm-cinder-dell-emc-powerstore config https://review.opendev.org/c/openstack/project-config/+/851242 | 17:00 |
| clarkb | you're welcome | 17:03 |
| ianw | elodillies: yeah, i will look into that, sorry about that | 20:47 |
| fungi | ianw: clarkb has a probable fix pushed | 20:49 |
| fungi | ianw: https://review.opendev.org/851239 | 20:49 |
| ianw | oh, excellent. sorry i see the scrollback now | 20:51 |
| ianw | it's kind of lame that "include_role" looks exactly like a task where "foo | default(omit)" would work, but it doesn't | 20:57 |
| ianw | i've written a fair bit of ansible and didn't realise unitl now; basically just a foot-gun you don't know until you hit it | 20:57 |
| ianw | (it being your foot :) | 20:58 |
| ianw | i wonder if ansible-lint could pick it up | 20:58 |
| *** timburke_ is now known as timburke | 20:58 | |
| opendevreview | Merged openstack/project-config master: Set ensure_pip_virtualenv_command in Focal proposal jobs https://review.opendev.org/c/openstack/project-config/+/851239 | 21:01 |
| *** dasm is now known as dasm|off | 21:02 | |
| jrosser | ianw: the problem with omit is you can set a var to it if the only thing you do is pass the var to a module | 21:20 |
| jrosser | but then inevitably the var gets used also for something else later which goes very badly | 21:21 |
| jrosser | imho the only place omit should be used is param: {{ condition | ternary(value, omit) }} | 21:23 |
| jrosser | or param: “{{ value | default(omit) }}” | 21:24 |
| jrosser | directly on a module, anything else is deserving a second look | 21:24 |
| ianw | jrosser: i guess though; "include_role: <foo> \n vars: {{ arg | omit }}" looks exactly like sending an argument to a module | 21:30 |
| jrosser | unfortunately that is true, it does look that way | 21:31 |
| jrosser | restricting it to being used only on module params makes it really clear when a param is omitted, because the condition ends up being right there with the parameter | 21:33 |
| jrosser | otherwise you have something that looks like a var which sometimes is the magic omit value, and that’s really tricky for whoever reads the code next to understand | 21:34 |
| *** rlandy is now known as rlandy|bbl | 22:06 | |
| *** dviroel is now known as dviroel|afk | 22:36 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!