Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-01-25
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 sean-k-mooney cdent: that can be true but when we have function like post_live_migration_at_destination(...)
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: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 your monitor is.
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: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 mriedem i mean there is no glance image in swap disks yeah?
14:08:20 mdbooth So it's basically a cache of the output of mkswap.
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/
14:19:05 mdbooth mriedem: I had to check closely, but I'm confident we don't age ephemeral backing files.
14:19:19 mdbooth reasonably confident :) That code is very hard to read.
14:20:10 mdbooth I believe ext3 was the default, might still be, and ext3 fs creation can be slow.
14:20:33 mriedem yeah i remember a ML thread from robert collins several years ago about changing the default to ext4
14:20:41 mdbooth I suspect that's how this feature came about. But note that we still have to do this for other imagebackends anyway, and ext4 fs creation is really fast anyway.
14:22:27 mriedem looks like ext4 is the default now
14:22:30 openstackgerrit Merged openstack/nova master: Add missing ws seperator between words https://review.openstack.org/632606
14:22:58 mriedem https://github.com/openstack/nova/blob/master/nova/privsep/fs.py#L258
14:26:33 kashyap mdbooth: (Our of curiosity, I ran your same test on an NVMe: http://paste.openstack.org/show/743468/)
14:27:16 mdbooth kashyap: That's consistent, although I doubt either of our tests were actually hitting disk.
14:27:27 mdbooth kashyap: Likely due to RAM/CPU speed differences.
14:28:17 kashyap Oh, indeed. I always forget that point
14:28:42 kashyap (I recall that `fio` is the better way to do benchmarks)
14:29:03 kashyap (Reference: http://blog.vmsplice.net/2017/11/common-disk-benchmarking-mistakes.html)

Earlier   Later