SpamapS | anyway... another thing biting me right now is the inability to require an OR condition, like 'Either it has an Approved review from a person with write access, _or_ it has the 'self-approved' label. | 00:00 |
---|---|---|
jeblair | SpamapS: triggers are or'd | 00:14 |
jeblair | SpamapS: so you may be able to add another trigger and do what you want. | 00:17 |
jeblair | SpamapS: and yeah, they have to share a queue. however, we might be able to have the zuul trigger emit an event when a dependent change merges which could be used as a trigger. | 00:17 |
*** jamielennox has quit IRC | 01:20 | |
*** persia has quit IRC | 01:21 | |
*** persia has joined #zuul | 01:22 | |
*** jamielennox has joined #zuul | 01:23 | |
*** jhesketh_ is now known as jhesketh | 02:22 | |
tobiash | SpamapS: you might want to add the zuul event parent-change-enqueued to your gate pipeline | 05:08 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Create job-output.txt together with JobDir https://review.openstack.org/514617 | 05:13 |
*** tushar has joined #zuul | 05:36 | |
*** xinliang has joined #zuul | 05:36 | |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Document executor/merger stats https://review.openstack.org/514343 | 06:02 |
*** umbarkar has joined #zuul | 06:04 | |
*** tushar has quit IRC | 06:04 | |
*** yolanda_ has joined #zuul | 06:12 | |
*** bhavik1 has joined #zuul | 06:13 | |
kklimonda | is there going to be a race between nodepool hold [node currentty in-use/locked] and zuulv3 freeing the node for deletion? | 06:39 |
*** yolanda_ has quit IRC | 06:40 | |
*** umbarkar has quit IRC | 06:40 | |
*** yolanda_ has joined #zuul | 07:42 | |
*** yolanda_ is now known as yolanda | 07:43 | |
*** bhavik1 has quit IRC | 08:19 | |
*** electrofelix has joined #zuul | 08:34 | |
*** rcarrill1 is now known as rcarrillocruz | 09:48 | |
*** jkilpatr has quit IRC | 11:11 | |
*** yolanda has quit IRC | 11:19 | |
kklimonda | so, some of my logs (output of the main playbook in job-output.txt.gz) are truncated, with traceback like that in executor-debug.log: http://paste.openstack.org/show/624588/ | 11:28 |
kklimonda | looking at the example log (https://logs.opencontrail.org/92/36592/2/check/contrail-vnc-build-unittest-ubuntu-xenial/6e973ee/job-output.txt.gz) I see that there are some characters that are probably unicode - given that UnicodeDecodeError is raised on the last byte of the buffer, I assume we are trying to decode unicode character that's split between reads | 11:30 |
*** jkilpatr has joined #zuul | 11:55 | |
tobiash | kklimonda: seems that you hit a bug in the log streaming code | 12:10 |
kklimonda | yup - either side should handle that, not sure which though | 12:10 |
kklimonda | either sender or receiver I mean | 12:11 |
tobiash | kklimonda: zuul decodes the log with utf-8 | 12:17 |
tobiash | kklimonda: you probably have some job which outputs binary data to stdout (which I think is a bad thing but should not crash zuul) | 12:17 |
kklimonda | tobiash: no, I have a job that's outputting utf-8 to stdout | 12:17 |
tobiash | kklimonda: well the log says it's not utf-8 | 12:18 |
tobiash | kklimonda: ah, maybe the buffer splits a multi byte character | 12:19 |
kklimonda | tobiash: what seems to be happening is that zuul is trying to decode an incomplete unicode.. code point (I think?) because it spans buffer boundary | 12:19 |
kklimonda | I wonder if that's related to zuul-web randomly dropping log streaming | 12:23 |
kklimonda | hmm | 12:23 |
openstackgerrit | Dirk Mueller proposed openstack-infra/zuul-jobs master: Remove pep8/pyflakes https://review.openstack.org/515041 | 12:27 |
*** yolanda has joined #zuul | 12:27 | |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Do late decoding of log stream buffer https://review.openstack.org/515043 | 12:28 |
tobiash | kklimonda: this could be a fix for this ^ | 12:29 |
kklimonda | tobiash: let me try it really quick, I seem to be hitting that issue pretty reliably | 12:29 |
tobiash | k | 12:29 |
tobiash | kklimonda: I'm not sure if split works with binary strings, if yes this should work hopefully | 12:30 |
kklimonda | tobiash: ha, I've been thinking about splitting on \n - I was a bit worried about what could happen if someone started feeding it logs with no \n characters | 12:31 |
tobiash | kklimonda: then it will not split the line and have it all on one line in the last log line at the end | 12:31 |
openstackgerrit | Andrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role https://review.openstack.org/509233 | 12:52 |
openstackgerrit | Andrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role https://review.openstack.org/509233 | 13:09 |
SpamapS | tobiash: Oh! Thanks, I'll try parent-change-enqueued | 13:11 |
tobiash | SpamapS: I had an issue that approving changes with depends-on in the wrong order left one change not entering the gate | 13:15 |
tobiash | SpamapS: then I discovered parent-change-enqueued :) | 13:15 |
SpamapS | tobiash: Indeed I think it's the right thing. Hopefully it crosses shared queue bounds. | 13:16 |
tobiash | SpamapS: I think with not shared queues the trigger will fire but the queue will reject it | 13:17 |
openstackgerrit | Andreas Jaeger proposed openstack-infra/zuul-jobs master: Silence ansible-lint https://review.openstack.org/514526 | 13:17 |
tobiash | SpamapS: in this case you probably need to listen on a merge event | 13:17 |
tobiash | SpamapS: but I might be wrong with the queues | 13:18 |
SpamapS | Well it's possible I want them shared. I'm not sure. | 13:18 |
SpamapS | but I feel like I don't. | 13:19 |
SpamapS | Probably need to read the docs again. | 13:19 |
tobiash | a use case for not having them shared would be e.g. nova and zuul-jobs | 13:19 |
SpamapS | I'm more concerned with how to get things into the gate from github.... using the reviews API won't allow self-merge.. using labels seems to have race problems. | 13:20 |
tobiash | one would still want depends on between them | 13:20 |
SpamapS | tobiash: exactly | 13:20 |
SpamapS | in my case I'm adding jobs and then adding usage of the jobs in several repos. | 13:20 |
tobiash | SpamapS: in this case you probably don't want shared queues | 13:21 |
SpamapS | right, but I do want to put the children in their gate when the parent merges | 13:21 |
tobiash | SpamapS: do we have a parent-change-merged event? | 13:22 |
tobiash | SpamapS: I think such a thing would be the thing we want for this use case | 13:22 |
SpamapS | agreed | 13:22 |
SpamapS | because the children won't enqueue on parent-change-enqueued | 13:23 |
SpamapS | since their queue will not have the parent ahead of them | 13:24 |
tobiash | SpamapS: looks like this is not existing yet: https://docs.openstack.org/infra/zuul/feature/zuulv3/admin/drivers/zuul.html | 13:24 |
tobiash | SpamapS: but I think it's useful | 13:24 |
SpamapS | tobiash: indeed I'm looking to see if we can generate it | 13:24 |
SpamapS | as long as we don't drop the needed_by_changes event I think we can do it relatively easily | 13:28 |
SpamapS | s/event/attribute/ | 13:28 |
tobiash | SpamapS: this attribute should be also used by the parent-change-enqueued so I doubt we drop this attribute | 13:30 |
SpamapS | tobiash: yeah I think it will be there. | 13:32 |
SpamapS | Unfortunately I don't know if I'll have time to work on that for a while. | 13:32 |
SpamapS | Until then we just have to toggle our approvals. | 13:32 |
*** jkilpatr has quit IRC | 13:56 | |
*** yolanda has quit IRC | 14:28 | |
openstackgerrit | Andrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role https://review.openstack.org/509233 | 15:02 |
openstackgerrit | Andrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role https://review.openstack.org/509233 | 15:30 |
jlk | SpamapS: we did put in a feature request for allowing admin self-review. | 16:00 |
*** hashar has joined #zuul | 16:04 | |
pabelanger | jeblair: mordred: did you want to look at 514526 and comment? Its about using skip_ansible_lint tags to make ansible-lint happy. Want to make sure everybody is okay with it, since they could be sprinkled around playbooks. | 16:04 |
SpamapS | jlk: I recall that we did, but don't remember if it was in the "totally no way" answer or the "mmk" answer category. | 16:04 |
SpamapS | jlk: they also gave us the "totally no way" answer on defaulting to dismissing reviews on push.. then a month later added the checkbox. ;) | 16:04 |
jlk | I had a conversation with review folks at github universe. They seemed pretty into allowing that | 16:05 |
jlk | since they already allow admins to override required review to merge, and other such things | 16:05 |
jlk | so I have some hope | 16:05 |
SpamapS | jlk: right, seems like having everything in the same API vs. just brushing past it would be idea. | 16:05 |
SpamapS | ideal | 16:05 |
SpamapS | jlk: until then... I'm not sure what to do. | 16:05 |
jlk | yeah :( | 16:05 |
jlk | labels :/ | 16:05 |
SpamapS | which has the delay issue for me | 16:06 |
jlk | right. | 16:06 |
SpamapS | though keeping zuul busier has helped with that ;) | 16:06 |
jlk | totally open to introducing a delay there | 16:06 |
SpamapS | I think it is literally a matter of 10ms | 16:06 |
jlk | I think we do some of that in gerrit too | 16:06 |
SpamapS | my guess is the webhook happens async in GH from the write to the db | 16:06 |
jeblair | yeah, gerrit is a continuous 5s behind | 16:06 |
SpamapS | orly? | 16:07 |
SpamapS | Another problem with the label approach is that only owners can apply labels. | 16:07 |
SpamapS | so I have to make somebody a repo owner to let them approve patches. That kind of makes sense.. but it's a bit of a mismatch in power. | 16:07 |
jeblair | yep. race conditions on the server mean that a query immediately after an event may return old data. so we put it into a queue and make sure 5s have elapsed before we pull it off. the effect is like zuul being 5 seconds in the past. which is weird, because zuul usually lives in the future. | 16:08 |
SpamapS | I noticed that Kubernetes uses a system where people comment '/lgtm' or '/approved' and a bot applies the labels after checking their membership. | 16:08 |
*** bhavik1 has joined #zuul | 16:08 | |
SpamapS | jeblair: Zuul: a time traveler for all your testing needs. | 16:08 |
SpamapS | Wait no, Gozer is the traveler. | 16:09 |
jeblair | the number of people privately employed to work around the fact that github is not open source is amazing. | 16:09 |
SpamapS | jeblair: :( | 16:09 |
SpamapS | Which capital to spend: Evangelize Zuul, Evangelize Gerrit, Evangelize Rust | 16:09 |
* SpamapS takes a shot | 16:09 | |
jeblair | pabelanger: looking | 16:09 |
jlk | SpamapS: I think you can grant somebody write access ( not owner ) so that they can apply labels. Also you can restrict WHO can actually commit to a branch down to just the zuul bot user (until you move to app based) | 16:10 |
jeblair | pabelanger: did you want to +3 that? | 16:11 |
pabelanger | jeblair: done, thank you | 16:13 |
* SpamapS looks at gerrit queue thing | 16:13 | |
tobiash | jlk: I had a github consultant here today and also asked the self review question | 16:23 |
tobiash | answer was nope, not possible | 16:24 |
jlk | sweet! | 16:24 |
jlk | waaat? | 16:24 |
jlk | that wasn't the answer we got :( | 16:24 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Create job-output.txt together with JobDir https://review.openstack.org/514617 | 16:24 |
tobiash | he said, doesn't make sense as the review override is the merge button for project admins... | 16:25 |
jeblair | the customer is always wrong | 16:25 |
tobiash | jlk: let's hope that you talked to devs more in that topic... | 16:26 |
jlk | that was the first response, but when we explained how review triggers CI, and how you can easily add extra contributors without changing settings then it made sense to them | 16:26 |
jlk | tobiash: the people I was talking to were devs | 16:26 |
tobiash | An I asked about github apps in enterprise | 16:26 |
jlk | what was the word on that? | 16:27 |
tobiash | it could be added as an early access feature to 2.12 | 16:27 |
tobiash | early access means that you have to enable this feature together with the support | 16:29 |
tobiash | (if I understood this correctly) | 16:29 |
tobiash | jlk: do you use labels for modeling the workflow flag? | 16:32 |
SpamapS | That resposne doesn't surprise me from a consultant. | 16:33 |
SpamapS | I've been a consultant in a software product company before. You get told no by every product manager you talk to, until you find it in the release notes because some customer went over your head and called the CTO. | 16:34 |
SpamapS | So your default answer to customers is usually "nope can't change {anything}" to keep expectations low. | 16:34 |
jlk | tobiash: So, if you can't do self review, we've made labels as a thing you could use to provide a workflow vote. | 16:36 |
jlk | tobiash: otherwise, the recommended method is to make use of pull request reviews. | 16:36 |
tobiash | jlk: does zuul already respect the branch protection settings (like for gerrit label) | 16:38 |
tobiash | Such that I don't have to require review on the pipeline config | 16:38 |
openstackgerrit | Merged openstack-infra/zuul-jobs master: Silence ansible-lint https://review.openstack.org/514526 | 16:39 |
tobiash | For gerrit it respects any additional mandatory label before entering the gate | 16:40 |
jlk | tobiash: not exactly no. | 16:41 |
jlk | tobiash: we have code to discover what branch protections are in place, and maybe we don't attempt to merge if it's not there? I forget how far down that path we got | 16:42 |
jlk | We wanted to get this in front of a bunch more github users before getting too complicated in the capabilities and whatnot | 16:42 |
tobiash | Maybe I'll have a look into this topic in the next few weeks | 16:42 |
jeblair | jlk, tobiash: GitHubConnection.canMerge has a big TODO and returns True | 16:43 |
jlk | hah yes. | 16:43 |
tobiash | (when I have a real github available for my v3 setup) | 16:43 |
jeblair | so i'm guessing no. but there are some thoughts in that comment about how to proceed :) | 16:43 |
jlk | I know jamielennox landed some code to inspect branch protection settings | 16:44 |
jeblair | actually, the way the comment is written, it almost looks like there maybe was code there before but got removed | 16:44 |
jlk | oh maybe it was an open PR | 16:44 |
jlk | er / change request | 16:44 |
*** hashar is now known as hasharAway | 16:45 | |
jlk | https://review.openstack.org/#/c/471175/ is what I was thinking of | 16:46 |
tobiash | btw, I'll be on vacation next week and currently a bit under pressure to get a v3 setup with openshift and github productive | 16:46 |
jlk | tobiash: okay, reach out if you get stuck at all. Happy to help! | 16:47 |
tobiash | That's why I'm a little less active here the last few weeks... | 16:47 |
jlk | so this change was mostly about other contexts that may be required (like if they use both travis and us, or whatever). We thought we could go deeper and check for other protection level things that could block us | 16:48 |
tobiash | jlk: thanks | 16:48 |
jlk | graphQL would make this easier, we could dig all that out in one shot rather than a pile of GET calls :/ | 16:48 |
jlk | really really really wish they'd turn on graphQL for apps | 16:48 |
tobiash | jlk: I also asked about graphql and the answer was that's also usable for apps | 16:49 |
tobiash | So either this works now or I got a wrong answer | 16:50 |
jlk | hrm, that's not advertised yet. At github Universe 2 weeks ago they said it's coming soon, but no commitment. | 16:50 |
jlk | https://developer.github.com/apps/ still says only REST v3 | 16:50 |
jlk | maybe it's in early access | 16:52 |
tobiash | Possibly | 16:53 |
SpamapS | it makes the REST calls and/or it gets the 429 | 16:54 |
jlk | Clint. | 16:54 |
jeblair | Jesse? | 16:56 |
* jeblair is not sure if he's playing right, but is trying | 16:56 | |
jlk | I was saying his name in disdain for his humor attempt. | 16:57 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add log streaming logging and exception handling https://review.openstack.org/513811 | 17:16 |
SpamapS | LEEEERROYY JENKINS | 17:16 |
Shrews | jeblair: reading your "branch issues" email, i've re-read "Parents should have variants applied" section umpteen times. And though I get why that's a problem, I'm not sure I grok the technical reason given: "But since the inheritance has already been applied, any variants which a user may expect to apply to a parent job will not be applied." | 17:18 |
Shrews | jeblair: if variants are applied dynamically, what does inheritence already being applied have to do with the problem? | 17:18 |
Shrews | i'm missing something (obviously) | 17:19 |
*** dmsimard is now known as dmsimard|afk | 17:23 | |
SpamapS | Shrews: I feel like I need a visual diagram for that one too | 17:25 |
SpamapS | It's all there | 17:25 |
SpamapS | But I think it has one more dimension than I can draw in my head. | 17:25 |
jeblair | Shrews: ah, it's because inheritance is determined when the config is loaded. so by the time zuul runs a job, it's too late to say "oh, this change is on stable/newton, and this job's parent, devstack, has a stable/newton branch variant, so it should apply too". | 17:27 |
Shrews | jeblair: "too late" means "past the point of knowing what the inheritance is"? | 17:28 |
jeblair | Shrews: so basically, if your job (say "tempest") has branch variants, you're fine. but if it's parent (say "devstack") does, you won't get them. your tempest job will have already picked a single devstack job to inherit from, when zuul was last reconfigured. it'll be the first one. | 17:28 |
jeblair | Shrews: yep. | 17:28 |
jeblair | Shrews: well, past the point of being able to do anything about it at least. | 17:28 |
*** bhavik1 has quit IRC | 17:29 | |
Shrews | jeblair: ah, I get it now. | 17:29 |
jeblair | Shrews: here's the code that has a job inherit from a parent job: http://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/configloader.py?h=feature/zuulv3#n505 | 17:30 |
SpamapS | "FAILURE in 52s" ... fail fast! | 17:50 |
openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Have tox-siblings edit requirements files instead https://review.openstack.org/514058 | 17:56 |
*** electrofelix has quit IRC | 17:57 | |
openstackgerrit | Clark Boylan proposed openstack-infra/zuul feature/zuulv3: Support cgit urls on Gerrit connections https://review.openstack.org/515168 | 18:36 |
openstackgerrit | Krzysztof Klimonda proposed openstack-infra/zuul feature/zuulv3: [WIP] Allow autohold on specific changes https://review.openstack.org/515169 | 18:36 |
clarkb | 515168 fixes an item on the zuulv3-issues etherpad, though it isn't really a zuulv3 issue so I've nto set the topic to zuulv3-fixes | 18:37 |
*** yolanda has joined #zuul | 18:40 | |
kklimonda | re 515169: I've seen requests from our developers to hold nodes for specific changes so that the failures can be investigated - I don't think that's quite possible right now, so I've started tinkering to see how that could be made to work. | 18:40 |
tobiash | clarkb: left a comment | 18:43 |
clarkb | tobiash: thanks | 18:43 |
clarkb | kklimonda: left some comments on that change | 18:47 |
openstackgerrit | Clark Boylan proposed openstack-infra/zuul feature/zuulv3: Support cgit urls on Gerrit connections https://review.openstack.org/515168 | 18:52 |
clarkb | tobiash: ^ fixed | 18:52 |
jeblair | clarkb: can you clarify one of your comments on 515169? (i left question inline) | 18:57 |
clarkb | jeblair: responded | 19:00 |
jeblair | clarkb: though change without uuid lets you set the autohold before the job starts | 19:01 |
clarkb | jeblair: oh thats a good point hrm | 19:01 |
jeblair | (also, uuid is risky in that it could be subject to cancelation. i think it's okay to add because if that's what you want, sure. we probably just shouldn't depend on it for that reason) | 19:02 |
clarkb | jeblair: re cgiturl comemnt, after lunch I'll make a stab at making it a template instead | 19:02 |
jeblair | clarkb: ok cool. i think i'd prefer that, but didn't want to "-1 refactor" :) | 19:02 |
jeblair | clarkb: lemme know if i can facilitate that :) | 19:02 |
jeblair | (the template stuff is getting more generic, so i don't think it'd be too hard; model objects are growing getSafeAttributes() methods) | 19:03 |
clarkb | ya should be straightforward just provide a template string that has access to base_url and project.name and sha. Then interpolate those as necessary (you might choose to not use base_url as in the cgit on different host case) | 19:04 |
clarkb | for autoholding I guess you'd have to allow change natively | 19:05 |
clarkb | for me at least I tend to not want all the jobs from a change and want a very specific nodeset (which build-uuid would support) | 19:05 |
clarkb | ok lunch now | 19:05 |
kklimonda | clarkb: won't passing --job to autohold limit request to only nodeset from the job? | 19:12 |
kklimonda | or are you talking of something else? | 19:12 |
clarkb | kklimonda: ya that will too, but that doesnt help the post/tag/release/periodoc situation so was trying to think of it from that perspective | 19:15 |
kklimonda | right | 19:16 |
jeblair | yeah, i think we should also add 'ref' for the post-merge situation | 19:18 |
kklimonda | re --ref, should that be a separate command to match enqueue-ref, or should I just add --ref [--oldrev] [--newrev] to autohold itself? | 19:33 |
kklimonda | also, initially my --change was actually --changeset, to allow for autoholding on the specific patchset, but I couldn't think of a reason to do that. | 19:36 |
jeblair | kklimonda: yeah, i like --change. i'd add --ref to autohold itself. basically it becomes a query with a bunch of 'and' conditions. adding oldrev/newrev could be useful, but if you do, i'd definitely make them optional (just matching on the ref without matching the values is useful -- much like matching the change without the patchset) | 20:12 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move test_model.test_job_inheritance_configloader https://review.openstack.org/509495 | 20:14 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove test_job_inheritance https://review.openstack.org/509509 | 20:14 |
kklimonda | jeblair: makes sense, thanks - I'll take a stab at it tomorrow. G'night :) | 20:18 |
jeblair | kklimonda: thanks, good night! | 20:20 |
*** tristanC has quit IRC | 20:43 | |
*** tristanC has joined #zuul | 20:52 | |
*** yolanda has quit IRC | 20:56 | |
*** hasharAway has quit IRC | 21:07 | |
*** dmsimard|afk is now known as dmsimard | 21:08 | |
openstackgerrit | Jeremy Stanley proposed openstack-infra/zuul feature/zuulv3: Default change and patchset to NULL in SQLReporter https://review.openstack.org/514423 | 21:58 |
*** xinliang has quit IRC | 22:20 | |
* SpamapS up to 5 zuul jobs and 3 nodesets now :-D | 22:25 | |
SpamapS | and 5 repos | 22:26 |
jamielennox | jlk, tobiash: that github protection code was working, i just hadn't written the tests for it and it was at a time where there was a fair bit of refactoring | 22:34 |
SpamapS | ultimately I just want a consistent experience | 22:36 |
SpamapS | the label provides the same experience whether you're self-approving or approving. | 22:36 |
jamielennox | also on that you should be following https://platform.github.community/t/repositories-which-have-protected-branches-with-push-restrictions-have-no-ability-to-grant-push-rights-to-integrations/1376 | 22:38 |
jamielennox | it's starting to get movement - although i wish it had "generated healthy internal debate" a few months ago | 22:39 |
SpamapS | hah yeah | 22:42 |
SpamapS | hm the bot isn't talking in here | 22:47 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: Add implied branch matchers on 'master' https://review.openstack.org/514459 | 22:51 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: experiment with late-binding inheritance https://review.openstack.org/511352 | 22:51 |
openstackgerrit | Jeremy Stanley proposed openstack-infra/zuul feature/zuulv3: Default change and patchset to NULL in SQLReporter https://review.openstack.org/514423 | 22:58 |
clarkb | jeblair: I got nerd sniped by something else. About to start working on the change url template thing now | 23:11 |
clarkb | jeblair: one problem with using a template like that is that we actually generate two different urls based on whether or not the sha1 is there | 23:40 |
clarkb | I guess I can see if that code path is ever actually hit | 23:41 |
clarkb | it uses event.newrev which I think will always exist for a ref updated event? it may be 0*40 though | 23:42 |
jeblair | clarkb: what's the path without the sha1? a change? or some specific kind of post-merge event? | 23:43 |
jeblair | clarkb: cause yeah, if it's change, then i think we use the connection-supplied url for that, since that's intrinsic. | 23:44 |
jeblair | (ie, gerrit itself) | 23:44 |
clarkb | ya if thee is a change number we do gerrit itself else we ask for the git url then in the function supplying that we check if the sha1 is not false | 23:44 |
jeblair | oh hrm. yeah i wonder what that is | 23:45 |
jeblair | maybe it was just paranoia? | 23:45 |
clarkb | we always pass event.newrev as the sha value | 23:45 |
clarkb | ya I am thinking it may have been unnecessary belts and suspenders | 23:45 |
clarkb | I'll work on a patchet assuming its not necessary so we have something more concrete to think about | 23:45 |
jeblair | ++ i can't think of anything else right now :) | 23:45 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: experiment with late-binding inheritance https://review.openstack.org/511352 | 23:46 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: Add implied branch matchers on 'master' https://review.openstack.org/514459 | 23:58 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: experiment with late-binding inheritance https://review.openstack.org/511352 | 23:58 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!