Closed jirutka closed 11 years ago
Looks good with the exception of the SSHKey immutability. I agree with the concept but would prefer not to make this change in v1. I've made a note to do this in v2.
Also, if you could give me some tests for the new objects/behavior.
Well, here is spec for DirtyProxy. GitoliteAdmin doesn’t have tests (its spec is empty) so I’d have to write it all from scratch, but I don’t have time to do that now.
Should I rebase my branch without the immutable patch so you’ll be able to merge it simply from GitHub?
That's fine on the GitoliteAdmin side of the testing.
Nope, I can cherry-pick it out. I'll get this merged later today.
Are you alive? :)
Ah sorry! Yes I am still alive. I totally spaced on this. I will do it today during the football game.
Merged. Thanks!
There’s big performance issue with saving changes in GitoliteAdmin. It takes seconds with hundreds SSH keys in the keydir (about 10 sec with 250 keys for me). The problem is that it’s always loading all keys from file system even when app doesn’t need them and then writing all the keys back to file system (very expensive operation).
I made two optimizations:
ssh_keys
andconfig
just when they’re needed, not during initialization. Don’t write them back when not loaded (i.e. changed).ssh_keys
hash and don’t rewrite keys which hasn’t been changed since load from files. My DirtyProxy is not “bulletproof”, it’s very simple, but good enough.I also made SSHKey immutable. It’s not necessary* but I highly recommend to make SSHKey immutable. If nothing else than because you’re using field
owner
as a hash key inssh_keys
so it should not be changed while being in hash under theowner
as a hash key.