| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Github - Require webhook_token https://review.openstack.org/488240 | 00:15 |
|---|---|---|
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Github - Require webhook_token https://review.openstack.org/488240 | 00:26 |
| *** harlowja has quit IRC | 01:10 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use mypy to do static type checking https://review.openstack.org/488161 | 03:58 |
| *** harlowja has joined #zuul | 04:32 | |
| *** jesusaur has joined #zuul | 05:00 | |
| *** harlowja has quit IRC | 05:28 | |
| *** harlowja has joined #zuul | 05:41 | |
| *** harlowja has quit IRC | 06:48 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Ensure tox-cover is non-voting https://review.openstack.org/487173 | 07:08 |
| *** amoralej|off is now known as amoralej | 07:15 | |
| *** hashar has joined #zuul | 08:15 | |
| *** openstackgerrit has quit IRC | 08:18 | |
| *** openstackgerrit has joined #zuul | 08:22 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Fix dynamic layout with regex approval filters https://review.openstack.org/488331 | 08:22 |
| tobiash | jeblair, clarkb: ^ fixes the deepcopy issue you had a few days ago | 08:22 |
| tobiash | jeblair, clarkb: this bit me today too as soon as I added a username approval filter to the gate | 08:23 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Ignore .mypy_cache https://review.openstack.org/488332 | 08:26 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Fix dynamic layout with regex approval filters https://review.openstack.org/488331 | 08:36 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Update SQL reporter to store results https://review.openstack.org/488221 | 08:52 |
| mordred | let's see if maybe that doesn't copletely blow up | 08:52 |
| tobiash | mordred: lgtm | 08:54 |
| tobiash | mordred: ups, overlooked something, see review | 08:59 |
| mordred | uhoh. looking | 09:02 |
| mordred | SpamapS: +A on 488240 - but clarkb and I both left inline comments that may be worth following up on | 09:02 |
| mordred | tobiash: oh good call! | 09:03 |
| tobiash | :) | 09:03 |
| mordred | also - update unit test :) | 09:03 |
| tobiash | mordred: don't know currently if NODE_FAILURE is the longest though | 09:03 |
| mordred | I can just make it a little longer ... | 09:04 |
| mordred | no need to be SUPER short there - it's a varchar after all | 09:04 |
| tobiash | mordred: yepp, no need to be that grasping there ;) | 09:05 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Update SQL reporter to store results https://review.openstack.org/488221 | 09:06 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Github - Require webhook_token https://review.openstack.org/488240 | 09:09 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Update docs to indicate app_key is a file https://review.openstack.org/488337 | 09:10 |
| *** rbergero1 is now known as rbergeron | 09:17 | |
| *** rbergeron has joined #zuul | 09:17 | |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Update SQL reporter to store results https://review.openstack.org/488221 | 09:30 |
| *** hashar has quit IRC | 09:42 | |
| *** jkilpatr has quit IRC | 10:34 | |
| *** jkilpatr has joined #zuul | 10:53 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Implement a static driver for Nodepool https://review.openstack.org/468624 | 11:43 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Implement an OpenContainer driver https://review.openstack.org/468753 | 11:43 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Move the fakeprovider module to the fake driver https://review.openstack.org/488383 | 11:43 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Refactor provider config to driver module https://review.openstack.org/488384 | 11:43 |
| *** hashar has joined #zuul | 12:29 | |
| *** dkranz_ has joined #zuul | 12:56 | |
| jeblair | mordred: so.... apparently we're doing type checking | 14:29 |
| mordred | jeblair: yah, I should have been more careful and put a WIP on that | 14:31 |
| jeblair | mordred: so what exactly does it get us? | 14:32 |
| Shrews | oh, did that merge? | 14:32 |
| mordred | well - right now it gets us checking of inferred types, which isn't super exciting... | 14:32 |
| mordred | but the bit that's more cool is that if we add type annotations to method signatures, it will cause those to actually be checked | 14:33 |
| jeblair | mordred: do you have an example of an app that does this? | 14:34 |
| mordred | one sec | 14:34 |
| jeblair | to me, that seems like the real question. we need to decide if we want to start doing that or not, because the patch as written isn't really a goal in and of itself. | 14:35 |
| mordred | yes - totally agree. the straw man was "being able to add type annotations can help sometimes" - definitely not "we can just run more tools on the current thing" | 14:37 |
| mordred | https://github.com/tomchristie/apistar/blob/master/apistar/routing.py#L158 here's an example from a project that decided to jump pretty heavily in to making use of 3.x features | 14:38 |
| jeblair | of course they're backwards :) | 14:39 |
| mordred | jeblair: they want SQL to feel loved :) | 14:39 |
| jeblair | mordred: what does this mean? initial_types: List[type]=None | 14:40 |
| mordred | that means that it takes a List which can contain anything that subclasses type - and that the argument initial_types has a default value of None | 14:42 |
| mordred | jeblair: so - you can see in a couple of lines that the things being put into initial_types are classes, not instances | 14:43 |
| jeblair | oh, weird. it was the =None that i find most confusing. | 14:44 |
| mordred | yah - I agree, that combo is a little strange | 14:44 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: DNM Random zuul type hint examples https://review.openstack.org/488442 | 14:45 |
| mordred | jeblair: there's a couple of small examples of applying to zuul - I picked a place where it wasn't clear at first on reading what the incoming data type and where it seemed the use of an annotation might communicate something | 14:47 |
| jeblair | mordred: what would the annotation for 'items' look likE? | 14:47 |
| jeblair | it's a list of dicts of (dicts and lists and scalars...) | 14:48 |
| mordred | let's see ... | 14:48 |
| mordred | jeblair: items: List[Dict[str, Any]] | 14:51 |
| mordred | jeblair: I believe should do the trick | 14:51 |
| clarkb | one thing that bugged me (and I may have misread) was that mypy inferrs Any in the vast majority of cases which isnt really a strong co tract. its like using void* everywhere | 14:51 |
| *** harlowja has joined #zuul | 14:51 | |
| jeblair | mordred: is List[Dict] sufficient, (or List[Dict[]] )? | 14:51 |
| mordred | jeblair: lemme see | 14:51 |
| mordred | jeblair: yes - it seems so | 14:52 |
| jeblair | mordred: also, i know there's static type checking with mypy -- is there any runtime checking in py3 itself? | 14:53 |
| mordred | jeblair: so we should be able to be as clear or as vague as we'd like | 14:53 |
| mordred | there is not | 14:53 |
| jeblair | okay, so we're not going to end up with "Exception: None passed in as items" | 14:53 |
| mordred | they have been implemented as informational annotations with the intent of things like mypy being able to process them - or for people to write metaclasses to do so if they want | 14:53 |
| mordred | no - not unless we got really clever with a metaclass somewhere | 14:53 |
| jeblair | that's never gone well | 14:54 |
| mordred | nope. well - there was that one time | 14:54 |
| mordred | I dont remember why we wrote that metaclass - but I do remember spending an entire day discussing its four lines of code | 14:54 |
| jeblair | i doubt we well ever be as smart as that again | 14:55 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: DNM Random zuul type hint examples https://review.openstack.org/488442 | 14:55 |
| mordred | jeblair: nope. we definitely won't | 14:55 |
| mordred | jeblair: that adds the items annotation just for reference | 14:55 |
| jeblair | well, i'm willing to give it a try. my main concerns are whether it's worth the extra effort, and whether it will be confusing to people or a turn-off (we're not tripping over zuul contributors at the moment, so i don't want to tell folks to go a way more than necessary). but.... | 14:57 |
| jeblair | (oh, and also whether now is the best time to make changes like this) | 14:58 |
| jeblair | if we're willing to say this is an experiment, let's try it out, see if it annoys or confuses us, see if we think it's worthwhile, don't feel compelled to immediately use it 100% and feel free to at some point in the future, rip it all out... okiedokie. i *do* like static type checking and miss it greatly. | 14:58 |
| jeblair | i think the mostly opt-in approach will help with that. | 14:59 |
| mordred | jeblair: ++ agree with all of that | 15:04 |
| Shrews | unless folks explicitly add the comments for types, it shouldn't affect our current workflow, right? | 15:06 |
| Shrews | or use the new func/method definitions | 15:07 |
| mordred | Shrews: that is correct | 15:08 |
| mordred | you'll need to -r your tox -epep8 run the next time to rebuild that and pick up the new depend - but other than that, no change | 15:08 |
| clarkb | unless you break some existing contract or inferred typing | 15:08 |
| clarkb | tox will pep8 job | 15:08 |
| clarkb | *fail pep8 | 15:08 |
| mordred | yah. and in those cases it'll hopefully be welcome information | 15:09 |
| mordred | jeblair, clarkb: https://review.openstack.org/#/c/488337/ doc update based on yesterday's learning about what the app_key option is | 15:10 |
| *** olaph has quit IRC | 15:16 | |
| *** olaph has joined #zuul | 15:20 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Update docs to indicate app_key is a file https://review.openstack.org/488337 | 15:21 |
| jeblair | tobiash: thanks for finding the approval deepcopy thing! | 15:23 |
| pabelanger | morning! | 15:25 |
| jeblair | mordred: left a question on 488221 (with a -1 that can be inverted depending on the answer) | 15:28 |
| SpamapS | mordred: doh, one less 0. :-P | 15:30 |
| jeblair | mordred, pabelanger: https://review.openstack.org/488216 pep8 job failed with a POST_FAILURE. (note that the upload did happen, it was an earlier post playbook -- unittests/post which failed) here's the executor logs: http://paste.openstack.org/show/616878/ and here's the upload: http://logs.openstack.org/16/488216/3/check/tox-pep8/c1d1c26/ | 15:33 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Ignore .mypy_cache https://review.openstack.org/488332 | 15:33 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fix dynamic layout with regex approval filters https://review.openstack.org/488331 | 15:33 |
| mordred | jeblair: hrm. that's a great question | 15:33 |
| jeblair | mordred, pabelanger: does the error "Ansible output: b'ERROR! A worker was found in a dead state'" mean anything to you? | 15:34 |
| mordred | jeblair: not even a little bit | 15:34 |
| jeblair | lib/ansible/plugins/strategy/__init__.py: raise AnsibleError("A worker was found in a dead state") | 15:35 |
| mordred | jeblair: ah - so, that's likely going to have something to do with the ssh subprocesses | 15:35 |
| pabelanger | jeblair: I haven not seen that before | 15:37 |
| jeblair | mordred, pabelanger: http://logs.openstack.org/16/488216/3/check/tox-pep8/c1d1c26/job-output.json it looks like the job output json does not include that playbook or anything after | 15:37 |
| jeblair | double check me on that | 15:38 |
| pabelanger | ya, I notice that last week, some playbooks were not logged into json | 15:38 |
| mordred | git.openstack.org/openstack-infra/zuul-jobs/playbooks/tox/post is the last one it ran | 15:44 |
| mordred | oh - yeah, that makes sense | 15:45 |
| mordred | if ansible-playbook aborts, the callback is never going to get the on_stats event | 15:45 |
| tobiash | jeblair: yeah, it broke my whole deployment as soon the approval username was set | 15:46 |
| tobiash | jeblair: and sorry for +3ing the static checker too quick, this probably would have required more consensus | 15:47 |
| jeblair | mordred: i guess the only playbook after that one is log uploading, which isn't there for obvious reasons. so i guess if we had one more post playbook, it probably would have been there. just coincidence that happened to be the last. | 15:47 |
| jeblair | tobiash: no worries. we can always revert stuff like that. but in this case, i think we can move forward and achieve consensus retroactively (or revert later) | 15:48 |
| *** harlowja has quit IRC | 15:51 | |
| *** amoralej is now known as amoralej|off | 16:04 | |
| SpamapS | zomg coffffffffffeeeeeeeeeeeeeeee | 16:09 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Ensure ref-updated jobs run with their ref https://review.openstack.org/488216 | 16:32 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Ensure ref-updated jobs run with their ref https://review.openstack.org/488216 | 16:38 |
| *** openstack has joined #zuul | 16:46 | |
| jeblair | alternatively, if folks aren't interested in reviewing docs changes, you could tell me to +3 my own changes | 16:46 |
| jeblair | mordred, pabelanger, clarkb, SpamapS: ^ | 16:47 |
| clarkb | jeblair: why is http://docs-draft.openstack.org/80/487580/2/check/gate-zuul-docs-ubuntu-xenial/32d8aab//doc/build/html/user/config.html#index-0 preferable to #Pipeline ? | 16:48 |
| clarkb | I think that comes from the switch to using index::\n single: Pipeline ? | 16:48 |
| jeblair | clarkb: i don't think it is preferable. we can drop it | 16:50 |
| jeblair | clarkb: fixed locally. i'll push it up after a rebase after the 2 parents merge since i need to resolve some conflicts on the children anyway. | 16:52 |
| clarkb | ok | 16:52 |
| *** harlowja has joined #zuul | 16:54 | |
| *** harlowja_ has joined #zuul | 16:55 | |
| pabelanger | jeblair: sure, let me look now | 16:57 |
| *** harlowja has quit IRC | 16:59 | |
| mordred | jeblair: I like reviewing docs patches - but also I'll say I'm completely comfortable with you self-approving docs patches too | 17:00 |
| *** hashar has quit IRC | 17:05 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul feature/zuulv3: Fix GithubConnection logging name https://review.openstack.org/488537 | 17:07 |
| pabelanger | jeblair: mordred: ^ I believe that fixes our debug logging for github connection | 17:07 |
| jeblair | oh the docs patches failed with this: 2017-07-28 16:48:01.730004 | zuul/sphinx/zuul.py:92: error: Need type annotation for variable | 17:09 |
| jeblair | so i guess i'll rebase the whole stack and figure that out | 17:10 |
| pabelanger | neat, openstack-zuul update the PR to pending: https://github.com/gtest-org/ansible/pull/1 | 17:25 |
| pabelanger | mordred: ^key worked properly | 17:25 |
| mordred | pabelanger: woot! | 17:26 |
| pabelanger | Ya, exciting | 17:26 |
| jeblair | apparently you can only see that if you're signed in? | 17:26 |
| pabelanger | ya, it looks that way | 17:26 |
| pabelanger | wonder if that can be changed | 17:26 |
| mordred | pabelanger, jeblair: I notice 2 bugs or possibilities for improvement | 17:27 |
| clarkb | jeblair: I wonder if non authenticed users get heavily cached content? | 17:28 |
| mordred | 1) the "Details" link in the pending job status is for "https://zuulv3.openstack.org/" - which isn't a thing | 17:28 |
| mordred | I _think_ that might be due to a setting in the app settings - but I wonder if it's a thing the plugin can report instead | 17:28 |
| jeblair | mordred: that may be the status-url which i asked pabelanger to leave blank for starters; i wanted to see the default | 17:28 |
| jeblair | but bonnyci configures it to be a status page deep link | 17:29 |
| mordred | jeblair: gotcha. so - in the app config on github there is a url for the service, which I believe is configured to https://zuulv3.openstack.org/ right now | 17:29 |
| mordred | so the default seems to be to use the value from the app config page | 17:29 |
| jeblair | mordred: *nod* | 17:29 |
| pabelanger | Ya, we are running defaults from zuul ATM | 17:30 |
| jeblair | mordred: i'm hoping we can work out a zuul github driver default so we don't have to set it in the pipeline config | 17:30 |
| mordred | jeblair: ++ | 17:30 |
| jeblair | i'm not sure if we have enough info yet to do that automatically, but i assume we will at least by the time we get all the web stuff ironed out | 17:30 |
| mordred | jeblair: so - the thing in the app config that's set to that url is "User authorization callback URL" | 17:30 |
| mordred | with a description of "The full URL to redirect to after a user authorizes an installation." | 17:31 |
| mordred | which doesn't seem to be the thing that this is | 17:31 |
| pabelanger | mordred: for now maybe we set openstack-zuul URL to http://zuulv3.o.o/status | 17:31 |
| mordred | so I'm not 100% sure why it would use that | 17:31 |
| jeblair | mordred: it may also be a default in zuul, i can't remember | 17:31 |
| mordred | ah - nod | 17:31 |
| mordred | maybe that's it - it doens't make sense for that value in github to be used in that location | 17:32 |
| jeblair | mordred: what else was on your list? | 17:32 |
| mordred | jeblair: I was going to suggest the bonny deep-link :) | 17:32 |
| *** openstackgerrit has quit IRC | 17:33 | |
| jeblair | cool, i guess we should try that now :) | 17:33 |
| mordred | it felt like, from that location, what I expected when I clicked on "details" would be the status details for that change, not the whole status page | 17:33 |
| pabelanger | cool, left comment about failure | 17:34 |
| pabelanger | URL to logs was correct | 17:34 |
| *** openstackgerrit has joined #zuul | 17:38 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Provide nicer index entries for config items https://review.openstack.org/487580 | 17:38 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add zuul:value sphinx directive https://review.openstack.org/487530 | 17:38 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Use default sphinx theme and index attributes https://review.openstack.org/487239 | 17:38 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove email-filter requirement https://review.openstack.org/487607 | 17:38 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Use zuul config sphinx directives for pipeline https://review.openstack.org/487604 | 17:38 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Cleanup pipeline requirements https://review.openstack.org/487618 | 17:38 |
| jeblair | okay that's the stack rebased and should by mypy clean | 17:38 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix gerrit label capitalization in docs https://review.openstack.org/488550 | 17:45 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix gerrit label capitalization in docs https://review.openstack.org/488550 | 17:48 |
| mordred | jeblair: did we get a mypy break in it? | 17:48 |
| jeblair | mordred: no, just grumped about an empty initializer. i removed it. | 17:48 |
| jeblair | it was something that other domains use, but the zuul domain doesn't use yet, but i had gone ahead and put it in anyway for reference. | 17:49 |
| mordred | nod. I'm interested as we hit grumps from it how often they're useful vs. nitpicky/lame | 17:50 |
| jeblair | mordred: this is along the lines of "unused variable". | 17:50 |
| jeblair | so, not exceptionally objectionable, but not that useful. | 17:50 |
| jeblair | (maybe not the best comparison -- unused variable is actually usually pretty useful because it may indicate a logic error; this was just an unused facility of a class) | 17:52 |
| tobiash | jeblair: I just discovered a glitch while playing with mypy: | 17:54 |
| tobiash | jeblair: http://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/driver/gerrit/gerritconnection.py?h=feature/zuulv3#n714 | 17:54 |
| tobiash | jeblair: this is calles with a project object but assumes a string | 17:54 |
| tobiash | jeblair: it works by accident though as the project object has a __str__ methog which does the right thing | 17:54 |
| jeblair | mordred: mypy scored a point ^ :) | 17:55 |
| tobiash | jeblair: now I'm not sure how far I should add the type annotations with the fix for that | 17:56 |
| jeblair | i think i'm getting pretty close with the ref-updated fix. it's currently failing some github tests becaues, in fact, the github test just makes up some random shas which aren't in the repo :) | 17:57 |
| jeblair | tobiash: maybe push up what you have and we'll see what it looks like? | 17:58 |
| clarkb | I think you give the function a signature then update the callers to pass a string not a project object? | 18:02 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Use project object in _uploadPack in gerrit driver https://review.openstack.org/488560 | 18:09 |
| tobiash | ^ did minimal annotations for the start as this is mainly a bugfix | 18:09 |
| pabelanger | Does git-review for github PRs exists? | 18:11 |
| pabelanger | or do people just keep using the webui? | 18:11 |
| *** openstack has joined #zuul | 18:15 | |
| mordred | oh - no, I don't really care about <3.5 | 18:16 |
| mordred | but other people could overrule me on that | 18:17 |
| mordred | the comment version for variables/attributes is needed in 3.5 because variable annotations don't hit until 3.6 | 18:17 |
| clarkb | mordred: you avoided using the 3.5 only features at least | 18:17 |
| clarkb | oh I thouht they were in 3.5 | 18:17 |
| mordred | no - that's 3.6 | 18:17 |
| tobiash | didn't we have already some py35 stuff (in zuul-web)? | 18:17 |
| mordred | and we do need to avoid 3.6 only features (sadly, there's a TON in 3.6 I like a lot - like being able to tell python that keyword arguments should never be able to be used positionally) | 18:18 |
| clarkb | mordred: python docs say 3.5? | 18:18 |
| mordred | clarkb: for variable annotations? (not function annotations) | 18:18 |
| clarkb | https://docs.python.org/3.5/library/typing.html | 18:18 |
| mordred | clarkb: yah - that's all function annotations | 18:19 |
| mordred | https://www.python.org/dev/peps/pep-0526/#abstract explains the difference | 18:20 |
| clarkb | I kind of think if we have to go the comment route for 3.5 anyways for a subset of cases then we should just be consistent and support other versions while we are at it | 18:20 |
| mordred | clarkb: well - I thnk the most common places where they're useful is in the parameters .. "what did this function take" | 18:21 |
| mordred | BUT - I could see doing it comment-style everywhere as well | 18:22 |
| clarkb | mordred: yes and that is supported in the comment method as well | 18:22 |
| mordred | class Example: | 18:22 |
| mordred | def method(self, lst, opt=0, *args, **kwargs): | 18:22 |
| mordred | # type: (List[str], int, *str, **bool) -> int | 18:22 |
| mordred | isn't terrible | 18:22 |
| clarkb | the other method of arg per line isn't bad either | 18:22 |
| clarkb | though it will add a lot of lines to code | 18:22 |
| clarkb | also maybe its just me but **bool makes no sense to me. kwargs should need two types because its a Dict? | 18:23 |
| clarkb | is the key side implicitly string because it will always be string? | 18:24 |
| mordred | I honestly don't know what the heck **bool is trying to communicate | 18:28 |
| clarkb | I'm guessing its equivalent to Dict(str, bool) ? since function arg names will always be str? | 18:29 |
| clarkb | but its confusing | 18:29 |
| mordred | clarkb: ah - no - it's the expected type for _one_ of the values | 18:30 |
| mordred | https://www.python.org/dev/peps/pep-0484/#arbitrary-argument-lists-and-default-argument-values | 18:30 |
| mordred | oh - nope - you were right | 18:30 |
| mordred | there is another doc that is wrong | 18:30 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Use project object in _uploadPack in gerrit driver https://review.openstack.org/488566 | 18:32 |
| *** jkilpatr has quit IRC | 18:32 | |
| tobiash | ^ is the alternative comment only version | 18:32 |
| mordred | oh - so - the downside to the comment only versoin ... | 18:32 |
| mordred | is that pep8 doesn't know the imported type has been used - so it emits an error for unused import | 18:33 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use default sphinx theme and index attributes https://review.openstack.org/487239 | 18:33 |
| mordred | whereas I believe that does not break if it's in the type annotation directly | 18:33 |
| tobiash | mordred: correct | 18:33 |
| clarkb | bah and the mypy docs say you have to have the import even if it is comment only | 18:33 |
| mordred | yah | 18:33 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add zuul:value sphinx directive https://review.openstack.org/487530 | 18:34 |
| mordred | so I say we do direct annotations for arguments so that we have the least amount of effort serving the pep8 gods | 18:34 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Provide nicer index entries for config items https://review.openstack.org/487580 | 18:34 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use zuul config sphinx directives for pipeline https://review.openstack.org/487604 | 18:34 |
| clarkb | I dunno I still think its worthwhile to not further restrict python versioning for something that is currently experimental ish | 18:35 |
| clarkb | I would have to completely rebuild my test bed | 18:35 |
| clarkb | maybe I already have to with the last patch? but I thought it was comment annotation only | 18:35 |
| tobiash | I'm fine with both but have a slight preference to annotations to arguments. This way the arguments and annotations directly match visually together especially for multiline function declarations. | 18:35 |
| clarkb | tobiash: mypy supports comments in multiline function declarations | 18:36 |
| mordred | clarkb: I mean- we don't test 3.4 so I'm not super comfortable declaring support for it anyway - and it's a new release so we don't really have legacy folks to be concerned with | 18:37 |
| mordred | clarkb: I don't feel STRONGLY one way or the other | 18:37 |
| mordred | mostly just listing my various thoughts | 18:37 |
| clarkb | if we didn't have to use comments anyways for 3.5 on variables I would agree | 18:37 |
| mordred | I'm happy either way we go | 18:37 |
| clarkb | but we do and have the same exact pep8 problem there | 18:37 |
| clarkb | since we don't avoid any of the overhead we may as well just make it work in more places | 18:38 |
| mordred | yah. I guess I'm mostly arguing that I doubt we'll annotate many variables | 18:38 |
| mordred | the one I had to do was just becaue the inferred type was wrong (variable in base class was initialized to None so the inferred type was NoneType but it should have been str) | 18:39 |
| mordred | method/function signatures and returns seem to me to be the most likely place any of this shows up frequently for us as a place we'll desire putting things | 18:40 |
| tobiash | ok, so now I have -1 on both patches ;) , so where do we want to go? | 18:40 |
| mordred | I mean - I say all that - but I'm gonna TOTALLY add this to shade with comment-based annotations - so this is more a "one feels better to me but the other is totally fine" thing | 18:40 |
| mordred | tobiash: I think we get jeblair to tie-break | 18:41 |
| tobiash | :) | 18:41 |
| *** openstack has joined #zuul | 18:46 | |
| mordred | jeblair: maybe let's just generally say "don't use variable annotations unless absolutely necessary" | 18:47 |
| jeblair | ya. for now at least. | 18:47 |
| mordred | ++ | 18:47 |
| tobiash | I think they are just necessary when initialized with [] or {} or similar | 18:48 |
| clarkb | tobiash: or the None situation that mordred ran into | 18:48 |
| tobiash | jepp | 18:48 |
| tobiash | when initializing with a concrete type or function return value the type seems to be autodetected | 18:49 |
| *** jkilpatr has joined #zuul | 18:49 | |
| clarkb | of course you could in some of the none situations assign to eg "" or 0 or other falsey matching value | 18:51 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Implement autohold https://review.openstack.org/486692 | 19:01 |
| jeblair | Shrews: there's no WIP on that!!!!11one1! | 19:03 |
| Shrews | jeblair: oh yeah. i mentioned things in #-infra | 19:03 |
| Shrews | too many channels | 19:03 |
| Shrews | at least it was an openstack channel :) | 19:03 |
| Shrews | jeblair: so, one thing about that is i pulled the node state from zuul.model rather than repeat the definition in zuul/zk.py. Once this merges, I'd like to do the same for the other states. | 19:08 |
| jeblair | Shrews: ok, i'll keep that in mind as i review | 19:10 |
| Shrews | hrm, zuul says the test fails to pass, but passed locally | 19:13 |
| Shrews | i can at least fix the pep8 fail | 19:15 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Implement autohold https://review.openstack.org/486692 | 19:16 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove email-filter requirement https://review.openstack.org/487607 | 19:16 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Cleanup pipeline requirements https://review.openstack.org/487618 | 19:17 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fix gerrit label capitalization in docs https://review.openstack.org/488550 | 19:18 |
| Shrews | jeblair: aha. rebasing on the latest code i now get the error for the new test | 19:25 |
| Shrews | KeyError: 'verified' | 19:26 |
| Shrews | wonder if i'm missing something from my yaml | 19:27 |
| Shrews | http://logs.openstack.org/92/486692/5/check/gate-zuul-python35/ccc094c/console.html#_2017-07-28_19_24_24_290010 | 19:30 |
| tobiash | Shrews: the labels are now case sensitive | 19:37 |
| tobiash | Shrews: you have to spell it 'Verified' | 19:38 |
| Shrews | tobiash: oh. that was, indeed, the issue. thx | 19:40 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Implement autohold https://review.openstack.org/486692 | 19:40 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Add project related type hints to gerritconnection https://review.openstack.org/488593 | 19:59 |
| *** olaph has quit IRC | 20:20 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Monitor job root and kill over limit jobs https://review.openstack.org/485902 | 20:22 |
| *** olaph has joined #zuul | 20:22 | |
| mordred | pabelanger: fwiw, I needed to make a patch to os_server for ansible/ansible - so I also submitted it as a PR to gtest-org/ansible so that we'd have a PR in there that actually testing against shade would be nice for | 20:23 |
| mordred | pabelanger: https://github.com/gtest-org/ansible/pull/3 | 20:24 |
| pabelanger | mordred: cool, I'll look at shade shortly | 20:28 |
| mordred | pabelanger: yah - I'm still down the stack a bit from it myself | 20:33 |
| jeblair | Shrews: i left some comments ranging from nits through things i think we should do in this or a future patch, to something i'm pretty sure we should do before landing that patch. they should be annotated. :) | 20:41 |
| jeblair | Shrews: i think your state variable change sounds good | 20:41 |
| jeblair | Shrews: and, reading tobiash's comments, looks like we have 2 votes for updatable counts. :) | 20:41 |
| Shrews | jeblair: awesome. will take a gander | 20:42 |
| jeblair | tobiash: did you see comment on https://review.openstack.org/486227 ? | 20:45 |
| Shrews | jeblair: all make sense. i'll make those changes. i also like using a 0 value to mean remove it | 20:45 |
| jeblair | Shrews: cool | 20:46 |
| mordred | jeblair: see scrollback to you in #openstack-infra | 20:52 |
| jeblair | mordred: ok, so python code then | 20:53 |
| jeblair | mordred: oh! | 20:53 |
| jeblair | you're talking about singleton arguments | 20:53 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fix broken project config in dynamic config test https://review.openstack.org/486226 | 20:53 |
| mordred | jeblair: yup | 20:54 |
| jeblair | mordred: yeah, i think that's fair. i'm also worn down enough that i'm okay removing those, unless it really makes a difference (like, a recursive algorithm depends on it or something). this is a step back from my previous position of defending them as long as they were correct. | 20:54 |
| mordred | jeblair: I actually LIKE them for the singleton use case | 20:55 |
| jeblair | ya | 20:55 |
| mordred | jeblair: but I think it's also a mistake enough that saying "# notabug singleton usecase" | 20:55 |
| mordred | can be helpful | 20:55 |
| mordred | (patch coming in a sec for the thing I just accidentally hit) | 20:56 |
| jeblair | i mean, i'm not going to go remove them actively. just saying if it's borderline ambiguous, maybe not worth the effort to defend. :) | 20:56 |
| jeblair | mordred: cool | 20:56 |
| tobiash | jeblair: yes, saw it, but didn't yet have time to work on that further | 21:01 |
| tobiash | jeblair: but I tried your suggestion with ProjectTemplateParser._parseJobList | 21:02 |
| tobiash | jeblair: and failed because there not the complete layout is included | 21:02 |
| tobiash | jeblair: but I saw another possibility | 21:03 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Replace singleton lists with None defaults https://review.openstack.org/488676 | 21:04 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Add an AFS keytab to the bind-mount if it's there https://review.openstack.org/488677 | 21:04 |
| mordred | jeblair: first one is the thing I think was unintentionally doing it | 21:04 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: WIP Don't ignore inexistent jobs in config https://review.openstack.org/488758 | 21:06 |
| tobiash | jeblair: ^ is my second ides (not finished yet) | 21:06 |
| tobiash | jeblair: that would validate the jobs on every layout generation | 21:07 |
| tobiash | jeblair: I think there are a few tradeoff decisions to be made | 21:08 |
| tobiash | jeblair: lazy validation has less (no) general runtime costs, but only covers jobs that are tried to run | 21:09 |
| tobiash | jeblair: where early validation implies runtime costs and have the risk of breaking overall zuul config when adding new repos or pushing changes through | 21:11 |
| jeblair | mordred: we shouldn't need to put afs into zuul itself i don't think | 21:11 |
| jeblair | mordred: there's a config option to set bubblewrap binds | 21:11 |
| jeblair | mordred: oh, you say you're going to add a generalized system for deployers to bindmount things in. it already exists. | 21:14 |
| jeblair | of course it's not documented | 21:15 |
| SpamapS | oh yay diskaccounting is happening | 21:16 |
| mordred | jeblair: yay for not documented! | 21:18 |
| mordred | jeblair: k. I will change that patch to update the docs :) | 21:19 |
| jeblair | ++ | 21:19 |
| mordred | jeblair: of course, the variable is ro_dirs | 21:19 |
| mordred | and in this case we want to bind-mount a file ... but whatev :) | 21:20 |
| jeblair | mordred: it is already plugged into puppet to, so should just need a system-config change for the puppet side | 21:20 |
| mordred | jeblair: neat! | 21:20 |
| jeblair | mordred: yeah, that was unclear to me, but i tested it the other day and either dir or file works | 21:20 |
| mordred | yup. I just tested the bubblewrap option | 21:20 |
| clarkb | maybe make it ro_path | 21:21 |
| jeblair | mordred: maybe we should change it to ro_bind and rw_bind | 21:21 |
| jeblair | or path | 21:21 |
| clarkb | (if worried about the confusion) | 21:21 |
| clarkb | bind works too | 21:21 |
| clarkb | ro_bind_path :) | 21:21 |
| jeblair | we're 2/2 for confusing so i think it may be worth it. :) | 21:21 |
| mordred | ++ | 21:21 |
| * mordred does that | 21:22 | |
| jeblair | w00t https://pypi.python.org/pypi/zuul-sphinx/0.1.0 exists | 21:22 |
| pabelanger | mordred: looks like zuul had some issues with https://github.com/gtest-org/ansible/pull/3 I see some execptions in debug.log. I'll dig into them in a bit and see what is going on | 21:24 |
| jeblair | neat. one of the scheduler tests verified that we can run post jobs on deleted refs. it did that by simulating deleting the master branch. | 21:26 |
| jeblair | with the patch i'm working on, i get the following error: | 21:26 |
| jeblair | 2017-07-28 14:25:13,994 zuul.AnsibleJob DEBUG [build: cd33a0ca40b44b63941f3df488fd4922] Sending result: {"error_detail": "Project org/project does not have the default branch master", "result" | 21:26 |
| jeblair | : "ERROR"} | 21:26 |
| jeblair | i think that test may need to be rethought. | 21:26 |
| jeblair | like -- what did you expect to happen? | 21:27 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Change name and document the bind_mount config paths https://review.openstack.org/488677 | 21:30 |
| mordred | jeblair: yah. I agree - that seems like a thing that needs to be rethought :) | 21:31 |
| jeblair | mordred, clarkb: now that i see that, i think making it plural might be important. so "trusted_ro_paths". ? | 21:32 |
| mordred | jeblair: sure - can do | 21:32 |
| jeblair | (sorry) | 21:33 |
| tobiash | jeblair: maybe we can discuss on monday about the pros and cons of lazy and eager validation of job existence | 21:35 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Change name and document the bind_mount config paths https://review.openstack.org/488677 | 21:36 |
| tobiash | have to eod now | 21:36 |
| tobiash | cya | 21:36 |
| jeblair | tobiash: ok have a good weekend! | 21:36 |
| tobiash | u2 | 21:36 |
| mordred | jeblair: remote: https://review.openstack.org/488859 Change bind mount option names to match zuul | 21:37 |
| mordred | piddle. that's totally not going to work because CI | 21:37 |
| mordred | silly testing | 21:37 |
| jeblair | mordred: needs pluralization | 21:37 |
| clarkb | oh I just approved | 21:37 |
| mordred | also needs expand/contract | 21:37 |
| mordred | it won't pass the gate | 21:37 |
| clarkb | 488677 | 21:38 |
| mordred | we consume that parameter in site.pp | 21:38 |
| jeblair | mordred: ack | 21:38 |
| clarkb | so we need to support both temporarily? | 21:38 |
| jeblair | clarkb: i think 77 is fine, we can push through on the puppet stuff | 21:38 |
| jeblair | clarkb: yup | 21:38 |
| jeblair | btw, a likely dvorak typo for pull_request is purr_request. just sayin. | 21:47 |
| mordred | jeblair: NICE | 21:54 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Ensure ref-updated jobs run with their ref https://review.openstack.org/488216 | 22:01 |
| jeblair | mordred: https://review.openstack.org/486698 wants you | 22:03 |
| mordred | actually, https://review.openstack.org/#/c/485897 wants in first - I'll rebase 486698 on top of it | 22:05 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Log an extra blank line to get space after each skip https://review.openstack.org/486698 | 22:06 |
| mordred | k. those no longer conflict | 22:06 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add spacer line after loops https://review.openstack.org/485897 | 22:16 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Add a short name to the project in the inventory https://review.openstack.org/488877 | 22:22 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Log an extra blank line to get space after each skip https://review.openstack.org/486698 | 22:23 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Change name and document the bind_mount config paths https://review.openstack.org/488677 | 22:30 |
| mordred | jeblair, clarkb: I missed a unittest before ^^ | 22:31 |
| jeblair | mordred: +3 | 22:33 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Add a short name to the project in the inventory https://review.openstack.org/488877 | 22:35 |
| mordred | jeblair: huzzah! | 22:36 |
| jeblair | mordred: ah just caught myself! i +2d your change then remembered docs. vars are documented in doc/source/user/jobs.rst | 22:40 |
| mordred | jeblair: oh! | 22:43 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove zuul:configobject https://review.openstack.org/488886 | 22:48 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Add a short name to the project in the inventory https://review.openstack.org/488877 | 23:20 |
| mordred | jeblair: this time with docs! | 23:20 |
| jeblair | mordred: w00t. re+2 | 23:23 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!