| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] Fix node request cleanup routine https://review.opendev.org/c/zuul/zuul/+/809970 | 06:44 | |
| -@gerrit:opendev.org- Felix Edel proposed: [zuul/zuul] Let zuul-web look up the live log streaming address from ZooKeeper https://review.opendev.org/c/zuul/zuul/+/809410 | 07:40 | |
| -@gerrit:opendev.org- Felix Edel proposed: [zuul/zuul] Let zuul-web look up the live log streaming address from ZooKeeper https://review.opendev.org/c/zuul/zuul/+/809410 | 12:09 | |
| @felixedel:matrix.org | corvus: Here is a small fix for the state watche in the job_request_queue: https://review.opendev.org/c/zuul/zuul/+/809629 | 12:29 |
|---|---|---|
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] UI: remove immutability-helper dependency https://review.opendev.org/c/zuul/zuul/+/809423 | 12:57 | |
| -@gerrit:opendev.org- Felix Edel proposed: | 13:40 | |
| - [zuul/zuul] Don't use executor.builds when processing build result events https://review.opendev.org/c/zuul/zuul/+/808091 | ||
| - [zuul/zuul] Don't use executor.builds to find out if tests are settled https://review.opendev.org/c/zuul/zuul/+/808792 | ||
| - [zuul/zuul] Remove the local builds list from the executor client https://review.opendev.org/c/zuul/zuul/+/809175 | ||
| - [zuul/zuul] Fix race in test_data_return_child_from_retried_paused_job https://review.opendev.org/c/zuul/zuul/+/808918 | ||
| @clarkb:matrix.org | felixedel: left a comment on that fix up | 15:40 |
| @jim:acmegating.com | Clark, felixedel: is it ever the case that event=None? | 15:45 |
| @clarkb:matrix.org | corvus: yes looks like reconnection or the first call to the watcher does not set event | 15:49 |
| @jim:acmegating.com | ah, event=None on reconnection | 15:49 |
| @clarkb:matrix.org | "it will only be set if the change to the data occurs as a result of the server notifying the watch there has been a change" | 15:49 |
| @jim:acmegating.com | Clark: i agree with your assesment :) | 15:53 |
| @clarkb:matrix.org | corvus: tobiash has a note on https://review.opendev.org/c/zuul/zuul/+/808841 and I'll try to rereview it myself this morning | 16:27 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] Move time database to SQL https://review.opendev.org/c/zuul/zuul/+/808841 | 16:33 | |
| @jim:acmegating.com | Clark, tobiash: ^ that should take care of the bug tobiash spotted (which was happily caught by a test) as well as the bug caught by the other test. | 16:33 |
| @jim:acmegating.com | fyi Clark, fungi, and i just tracked down a bug in #opendev related to merge jobs in zk. tl;dr is a merger crashed and the merge job cleanup process worked, but it didn't notify the scheduler (via a completed event) that it needed to do something, so it's still waiting for the job. | 17:21 |
| @jim:acmegating.com | i'm going to work on a solution | 17:21 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] Send synthetic merge completed events on cleanup https://review.opendev.org/c/zuul/zuul/+/810092 | 17:34 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] Send synthetic merge completed events on cleanup https://review.opendev.org/c/zuul/zuul/+/810092 | 17:38 | |
| @jim:acmegating.com | Clark: ^ something like that | 17:38 |
| @clarkb:matrix.org | Thanks looking shortly | 17:38 |
| @jpew:matrix.org | Is there an example of using zuul to test a submodule subproject by building the parent project? | 17:51 |
| @clarkb:matrix.org | corvus: I left a comment but +2'd if you can take a look to dobule check my note | 17:53 |
| @clarkb:matrix.org | jpew: The gerrit zuul may have something like that as they use submodules pretty extensively | 17:53 |
| @jpew:matrix.org | Clark: Sorry, do you happen to have a link? I can't find anything that might be "gerrit zuul" :/ | 18:00 |
| @clarkb:matrix.org | jpew: the Zuul that is run for Gerrit repos: https://gerrit.googlesource.com/zuul/config/ https://gerrit.googlesource.com/zuul/jobs/ https://gerrit.googlesource.com/zuul/ops/ (that last one is probably least likely to have what you are interested in it) | 18:02 |
| @jpew:matrix.org | @clark Got it thanks | 18:03 |
| @jpew:matrix.org | I missed that; the only links I found for gerrit were to a gerritforge CI | 18:03 |
| @jpew:matrix.org | (running Jenkins) | 18:03 |
| @clarkb:matrix.org | corvus: left some comments on https://review.opendev.org/c/zuul/zuul/+/808841 also CI seems unhappy with it | 18:55 |
| @jim:acmegating.com | Clark: thanks, replied. i'm not sure what's up with the tox-remote job... it does look like a related failure, but i don't see any clues about how | 19:08 |
| -@gerrit:opendev.org- Andrii Ostapenko proposed: [zuul/zuul] Treat 'waiting' jobs same as 'queued' in buildset progress bar https://review.opendev.org/c/zuul/zuul/+/803866 | 19:11 | |
| @clarkb:matrix.org | corvus: fwiw didn't meant to nitpick the filter vs comprehension. Noticed that sum() was used in the next block and thought maybe you'd prefer to use consistent functional methods there. Both are readable to me | 19:11 |
| @jim:acmegating.com | Clark: i may just be broken from the fact that python seems to be in a constant tug-of-war between different functional factions. :) | 19:13 |
| @jim:acmegating.com | anyway, we can't compose the sum line since times is used twice... at least, not without introducing a mean function, which there probably is in math but i was too lazy to look it up | 19:14 |
| @clarkb:matrix.org | Oh neat 803866 addresses something I've often wondered about. I can approve that one after I've validated teh behavior in the preview site | 19:15 |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] [web] Early support for pagination in builds, buildsets search https://review.opendev.org/c/zuul/zuul/+/808041 | 19:52 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] [zuul-web] Add pagination information when querying builds, buildsets endpoints https://review.opendev.org/c/zuul/zuul/+/808042 | 19:53 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] Pagination: remove compatibility code https://review.opendev.org/c/zuul/zuul/+/808043 | 19:53 | |
| @mhuin:matrix.org | hello zuul-maint, the pagination patch chain should be ready for reviews now: https://review.opendev.org/q/topic:%22ui_filtering_and_pagination%22+(status:open%20OR%20status:merged) - currently unverified patches were just rebased | 19:57 |
| @jim:acmegating.com | Clark: since you nerd-sniped me: https://paste.opendev.org/show/809445/ -- is that what you had in mind? | 20:18 |
| @clarkb:matrix.org | corvus: close for fitler if the function is None then it returns truthy values and excludes falsey values iirc. Not sure if that would be quicker or not though | 20:19 |
| @jim:acmegating.com | Clark: like https://paste.opendev.org/show/809446/ ? | 20:19 |
| @clarkb:matrix.org | just t = filter(None, times) I think | 20:20 |
| @clarkb:matrix.org | I'm impressed that list comprehensions seem to beat built ins though :) | 20:20 |
| @jim:acmegating.com | Clark: ah, filter(None...) is much faster, however it also excludes 0 which is not ideal for this use | 20:22 |
| @clarkb:matrix.org | Oh right we'd want to include 0 against the average | 20:22 |
| @jim:acmegating.com | Clark: that comes in at 0.16, so about 50% execution time as listcomp | 20:22 |
| @clarkb:matrix.org | got it, in the case where filter(None, l) works it is faster than a list comprehension otherwise use the comprehension | 20:23 |
| @jim:acmegating.com | yeah.... i will file that one away :) | 20:23 |
| @jim:acmegating.com | The more you know... 🌠| 20:24 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] Move time database to SQL https://review.opendev.org/c/zuul/zuul/+/808841 | 20:31 | |
| @jim:acmegating.com | okay, i think we're masking an exception that we shouldn't. i expect that to fail the tox-remote test again, but we should get an error this time | 20:31 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed on behalf of Felix Edel: [zuul/zuul] Fix wrong if condition in job request state watch https://review.opendev.org/c/zuul/zuul/+/809629 | 20:48 | |
| @jim:acmegating.com | Clark: ^ i went ahead and addressed your comment | 20:49 |
| @clarkb:matrix.org | +2 | 21:09 |
| @clarkb:matrix.org | https://review.opendev.org/c/zuul/zuul/+/803866 will need to be rechecked due to a failure talking to the npm registry | 21:11 |
| @jim:acmegating.com | okay, got the error for the sql change: pymysql.err.OperationalError: (1071, 'Specified key was too long; max key length is 3072 bytes') | 21:16 |
| @jim:acmegating.com | that's really weird to see it in the tox-remote job but not the unit tests; i wonder if there's a different mysql version in use or something | 21:17 |
| @jim:acmegating.com | character encoding could be an issue here too; the key is 1080 characters, but 4080 bytes in utf-32 | 21:18 |
| @jim:acmegating.com | * character encoding could be an issue here too; the key is 1020 characters, but 4080 bytes in utf-32 | 21:18 |
| @jim:acmegating.com | i think tox-remote runs on focal, and the others run on bionic | 21:26 |
| @clarkb:matrix.org | if it is utf8mb4 or whatever mysql calls the encoding then that allows for up to 4 bytes per utf8 character | 21:27 |
| @jim:acmegating.com | yeah; i think even utf8 may require 3 bytes | 21:27 |
| @jim:acmegating.com | which, with a few bytes of overheard, would also be above the limit | 21:27 |
| @clarkb:matrix.org | does this only affect the primary key index or all indexes? | 21:28 |
| @clarkb:matrix.org | I wonder if we can get away with having an index on the id column and then a secondary index on the other grouping? | 21:28 |
| @jim:acmegating.com | ostensibly, this is a secondary index, but it's also a unique constraint, so i wonder if those are subject to the same restrictions as a primary key | 21:29 |
| @clarkb:matrix.org | looks like it is both | 21:29 |
| @clarkb:matrix.org | https://wellingguzman.com/notes/mysql-key-limit seems to cover this well | 21:31 |
| @jim:acmegating.com | wait, does that say that the max length changes based on the charset? | 21:33 |
| @clarkb:matrix.org | no, the number of characters you can have depends on the charset due to the limit | 21:34 |
| @jim:acmegating.com | if that's the case, then i no longer have a hypothesis for why tox-remote raises the error but nothing else does | 21:34 |
| @clarkb:matrix.org | the limit changes based on server options and db type | 21:34 |
| @clarkb:matrix.org | they are saying utf8 gives you 767/3 or 3072/3 and utf8mb4 gives you 767/4 or 3072/4 for innodb depending on the options | 21:34 |
| @clarkb:matrix.org | In this case we are doing 255 * 4 = 1020 characters then encoding determines the limit. | 21:36 |
| @clarkb:matrix.org | utf8 would be under the limit at 3060, but utf8mb4 would not be? | 21:36 |
| @jim:acmegating.com | i think there's some overhead | 21:37 |
| @clarkb:matrix.org | I wonder if focal is defaulting to utf8mb4 but our docker-compose for local testing and bionic are defaulting to utf8? | 21:38 |
| @jim:acmegating.com | sounds plausible | 21:38 |
| @clarkb:matrix.org | https://mariadb.com/kb/en/innodb-limitations/#page-sizes mariadb has similar limitations so not a difference between mysql and mariadb I don't think | 21:39 |
| @jim:acmegating.com | apparently we set the default storage driver to myisam for tests | 21:41 |
| @jim:acmegating.com | and we create the database with charset utf8 | 21:42 |
| @clarkb:matrix.org | corvus: I think the idea with that is that we're supposed to force innodb in the model/schema/migrations? Also myiasm has lower limits | 21:42 |
| @jim:acmegating.com | (in tools/test-setup.sh) | 21:42 |
| @jim:acmegating.com | yeah, an unfortunate artifact of our openstack legacy :/ | 21:42 |
| @clarkb:matrix.org | https://dev.mysql.com/doc/refman/8.0/en/charset-unicode.html indicates mysql 8.0 (what is on focal) distinguishes between mb3 and mb4 utf8 charsets | 21:47 |
| @clarkb:matrix.org | I suppose ubuntu or debian may have updated some default though? | 21:48 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] Send synthetic merge completed events on cleanup https://review.opendev.org/c/zuul/zuul/+/810092 | 22:11 | |
| @jim:acmegating.com | Clark: bionic (and the docker local test setup (which i use but tox-* does not), and i'm guessing probably also zuul-quick-start) all default to latin1. focal defaults to: ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb40900ai_ci | 22:46 |
| @jim:acmegating.com | test-setup.sh declares 'character set utf8' when creating openstack_citest, but the actual unit tests create their own databases with no options specified. | 22:47 |
| @jim:acmegating.com | i think the shortest path out of this (assuming we don't want to make changes to the schema) is to adjust the test fixtures to specify utf8 when creating the ephemeral test databases. | 22:48 |
| @jim:acmegating.com | however, we have inadvertently been testing mysql+myisam+utf8mb4, and this would break that. i don't believe we have ever actually specified how we expect people to create databases.... | 22:48 |
| @jim:acmegating.com | i'd be happy to change the schema... but i chose that schema because it should produce consistent results from the postgres query planner. like, it should be nearly impossible for it to get wrong. i don't see how to do something like that without the unique constraint... | 22:54 |
| @jim:acmegating.com | we could do tenant/project/branch in their own tables and do a bunch of joins (and hope pg does that efficiently) | 22:58 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!