Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-01-25
12:39:43 sean-k-mooney i think its testing <80 instead of <=80
12:39:56 sean-k-mooney we dont explcitly set it
12:41:49 jangutter sean-k-mooney: 79 is clearly better because it's a prime number. 80 just has too many factors and it's not a pure power of two either.
12:43:33 sean-k-mooney true but in that case i vote for rounding up not down :)
12:44:53 jangutter sean-k-mooney: 80's possibly a holdover from 4:3 aspect ratio, but it still makes sense for people holding their phone in portrait mode.
12:45:21 jangutter sean-k-mooney: that's actually an argument to go for a 60 column limit.
12:45:44 sean-k-mooney there is also an argument to got to 160 or 190 the fortran uses
12:47:18 sean-k-mooney pep8 package doc example seam to encurate 160 indretly https://pep8.readthedocs.io/en/release-1.7.x/intro.html#configuration
12:48:49 jangutter sean-k-mooney: I just had a fringe idea - what if 79 characters is because there's a secret conspiracy to allow CRLF line endings?
12:49:22 sean-k-mooney they are both non pritable chariters so it does not matter
12:49:49 sean-k-mooney anyway i need to go rebase some things and fix some typos :)
12:50:29 jangutter sean-k-mooney: without nits like these to pick on Fridays, the world would be a lot more boring. Good luck!
12:51:24 cdent the world is unable to avoid this conversation isn't it
12:51:41 cdent I get unhappy when code lines are longer than 50 characters or so.
12:52:51 cdent My standards have completely collapsed since joining the openstack scene
12:55:49 jangutter cdent: I notice you didn't specifically say _coding_ standards.
12:56:02 cdent jangutter: that's correct
12:56:32 sean-k-mooney cdent: i hate wasting vertical space it almost -2 able in my book
12:56:51 cdent sean-k-mooney I WILL FIGHT YOU OVER THIS :)
12:56:52 sean-k-mooney its not but it severly hurts at leas my readablity of the code
12:57:23 sean-k-mooney cdent: :)
12:57:26 cdent I tend to associated wide code with not extracting methods enough
12:57:50 jangutter cdent: one more level of indirection can't hurt, right?
12:58:10 openstackgerrit Stephen Finucane proposed openstack/nova stable/rocky: Fix typo https://review.openstack.org/633196
12:58:11 openstackgerrit Stephen Finucane proposed openstack/nova stable/rocky: Fix overcommit for NUMA-based instances https://review.openstack.org/633197
12:58:20 cdent but interestingly it also has a lot to do with readability for me too: if I have to go to the right I get confused. sean-k-mooney if we combined our brains we'd either be able to read nothing or everything, not sure
12:58:20 sean-k-mooney cdent: that can be true but when we have function like post_live_migration_at_destination(...)
12:58:42 sean-k-mooney i can sometimes be hard to call them with out it requreing multiple lines
12:59:02 stephenfin lyarwood: Any chance you could you take a look at those two backports today?
12:59:06 jangutter cdent: I tend to notice something's wrong when the member I'm looking up gets to about 50 characters.
13:00:18 sean-k-mooney cdent: it only becomes annoying to me or hard to read when a function has multiple argumetns and i can nolonger clearly see everything returns and accpes at once
13:00:46 sean-k-mooney if the lines are too short or too long it become an issue but its also an issue if we have lost of indetation
13:01:35 sean-k-mooney for example in conditional within a fucntion within a fuction within a class.
13:02:07 sean-k-mooney in which case the satment may be only 30 letters wide but its wrapped because of the nested ness of the code
13:03:28 sean-k-mooney i think the best evolutionin in modern programming was clang-format
13:03:53 jangutter I've seen code (and yonks ago, written) with indents being 2 spaces. Doing it once makes you forever want 4 or more.
13:03:54 sean-k-mooney it inspired go to creat go formate and decide its a tools proplem to make the code consitent and readable
13:04:21 sean-k-mooney jangutter: i think the kernel still has an intend of 8 to discurabe deep nesting
13:04:53 jangutter sean-k-mooney: yep, indents shall be 1 tab and tabs are 8 characters wide like $deity intended.
13:05:13 sean-k-mooney yaml uses indentaion of 2 spaces i think by convention
13:17:26 mdbooth sean-k-mooney: I think 80 is arbitrary these days, but having a width limit around there does make sense even with very wide monitors, as it allows you to put multiple files next to each other. Most lines are not that long, so if you allow the occasional one to be that long you end up with truncation when viewing multiple files. Basically: imposing a (arbitrary) limit makes the code denser on screen, which is good no matter how big
13:17:26 mdbooth your monitor is.
13:17:46 mdbooth My 2c.
13:18:47 sean-k-mooney mdbooth: well im not saying we sure raise it even if i personally would it jsut annoys me that wehn reviewing in gerit or using an ide that set the 80 colum marker that the ci is actully enforice 79
13:20:04 sean-k-mooney you look at it either locally or in gerrit and it looks like it does not cross the virtical line that is rendered but actully its still rejected
13:20:08 mdbooth sean-k-mooney: I have vim set to show modifications in the margin, so my terminals all have to be 82 chars anyway :) I don't care what the actual value is, tbh, as long as it exists and is consistent.
13:20:49 sean-k-mooney ya its why i have never bother to "fix" it
13:21:00 mdbooth Ok, yeah, I can see that would be annoying if all the tools default to 80.
13:21:17 lyarwood stephenfin: ack will do
13:39:34 stephenfin TIL the 79 character default is in flake8, not tox.ini/setup.cfg
13:40:10 stephenfin sean-k-mooney: Fancy looking at https://review.openstack.org/#/c/631053/ and predecessors again today/early next week?
13:40:54 jangutter And 79 is actually codified in pep8 itself https://www.python.org/dev/peps/pep-0008/#maximum-line-length
13:40:56 sean-k-mooney sure i was just looking at your chages for requiring the plugin atribute for os-vif and the noop plugin
13:42:00 sean-k-mooney jangutter: ya they choose 79 so that if you have emacs in whitespace-newline-mode where it add a $ to mark the end of the line it wont wrap
13:42:16 jangutter think if we type 79 enough, 80 is going to look weird to sean-k-mooney?
13:43:00 sean-k-mooney jangutter: but all modern tools use 80 as teh defulat so its annoying
13:52:23 lyarwood stephenfin: https://review.openstack.org/#/c/633197/1/nova/objects/numa.py - stupid o.vo question in there if you have a second, I assume it's fine just wanted to check.
13:53:38 sean-k-mooney lyarwood: it should not no
13:54:10 sean-k-mooney lyarwood: the function signitru is part of the class deffinion not the serialised object
13:55:03 stephenfin lyarwood: Yeah, what Sean says. The scheduler, for example, will use its own version of the object
13:55:06 lyarwood sean-k-mooney: right, wasn't sure so it was worth asking. Thanks.
13:55:09 stephenfin *definition
13:55:27 sean-k-mooney lyarwood: its worth asking but no we are fine
13:58:48 sean-k-mooney this would matter for signiture of rpc methods where we are invokeing a remote method but not in this case
13:59:01 stephenfin lyarwood: I want tooling to automatically add (and remove, if abandoned) patches submitted to internal Gerrit to their BZ
13:59:15 stephenfin ...and reject the patch if there is no BZ linked
13:59:38 sean-k-mooney stephenfin: sound like a nice proejct. to discuss downstream ...
13:59:45 stephenfin Goddddd damn it
14:00:24 stephenfin Good thing I don't work in finance
14:00:28 sean-k-mooney i mean if someone upstream whats to write that for use tye are wellcome. im sure jangutter has plenty of time to do it:)
14:03:28 jangutter sean-k-mooney: if someone writes that, EVERYONE's going to pile on stuff to the weirdest BZ's.
14:04:44 jangutter stephenfin, sean-k-mooney: the OpenContrail project has something like that. The unintended consequence is that BZ quality goes down :-(
14:05:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Change InstanceFaultRollback handling in _error_out_instance_on_exception https://review.openstack.org/633212
14:05:28 mriedem mdbooth: i think you're familiar with _error_out_instance_on_exception - ^ is something that's always bugged me
14:05:44 openstackgerrit Matthew Booth proposed openstack/nova master: WIP: Don't age swap backing files in the image cache by default https://review.openstack.org/633213
14:06:02 openstackgerrit Merged openstack/nova master: Kill @safe_connect in _get_provider_traits https://review.openstack.org/613613
14:06:05 mdbooth mriedem: Looking, but ^^^ while you're here (discussion in bug: thanks for that)
14:07:28 mriedem i didn't really understand what swap disks have to do with the image cache manager
14:07:37 mdbooth mriedem: Right.
14:07:52 mriedem it's blank right? so why does the *image* cache manager worry about it?
14:08:04 mdbooth mriedem: It's not blank, it's formatted.
14:08:20 mdbooth So it's basically a cache of the output of mkswap.
14:08:20 mriedem i mean there is no glance image in swap disks yeah?
14:08:31 mdbooth No, there's no glance image.
14:09:01 mdbooth It saves a completely inconsequential amount of time during creation, and is likely a performance hit at runtime.
14:09:07 mdbooth And will save essentially no disk space.
14:09:45 mdbooth So we really shouldn't do it all, but cleaning that up would be a significant task. We'd need an upgrade task to rebase disks in use which already use this.
14:10:24 mdbooth However, aging these cached formatted swap files is really unlikely to be of much use to anybody.
14:10:40 mdbooth This user has found it's really expensive, so my suggestion is just don't age them.
14:11:37 mdbooth mriedem: Anyway, gimme a shout if you think ^^^ is an approach you could get behind and I'll fix that patch up properly.
14:13:09 mriedem i just don't really understand the use case for why these are in the image cache
14:13:22 mriedem granted, i'm not super familiar with the image cache manager to begin with
14:13:48 mriedem beyond it's used to cache and (on a timer) age/invalidate images in the cache so new versions can be fetched
14:14:06 mriedem but i thought that was only for root disks
14:14:14 mdbooth mriedem: I don't know the origin, so I can only guess. However ephemeral filesystem are also in there, and there are certain filesystems which *are* expensive to initialise.
14:14:24 mdbooth My guess is swap was simply handled the same way.
14:14:55 mdbooth Although to be clear, I would remove ephemeral filesystems too. We're already not aging them.
14:17:13 mriedem i don't see where ephemerals are in here
14:18:00 openstackgerrit Balazs Gibizer proposed openstack/nova master: Fix ComputeNode ovo compatibility code https://review.openstack.org/631199
14:18:23 mdbooth ext3 vs ext4 fs creation comparison: http://paste.openstack.org/show/743462/

Earlier   Later