#3094 Allow optimizing gitolite operations by using compile-1
Merged by pingou. Opened by bkabrda.
bkabrda/pagure gitolite-compile-1  into  master

Download 3094.patch

I'm still testing this PR, so please consider it a WIP. I'd also appreciate if someone else could actually test it before merging, since I don't consider this to be a trivial change.

rebased onto 555c83fd3c9ecc9c2ab6417b35c2f5c1f1bed531

Is there a minimal release version we could require/indicate?

Well, the newest version of compile-1 hasn't been in a release at all yet and neither the patch with ALLOW_ORPHAN_GL_CONF, so I can do that as soon as they're released, but the commits will have to do for now, I'm afraid.

ok, thanks

Does this drop the use for the cache file?

Well, the cache file is really only used on full gitolite compile, so this goes around it and doesn't touch it. Obviously, most pagure commands will still run the full gitolite compile. At that point, gitolite won't find the entry in the cache file and will regenerate the gl-conf for the repo. This shouldn't hurt though, because it will be the same. I'm still testing this to see if everything works fine, so please definitely don't merge :)

Stupid idea, instead of relying on a new configuration key, what about making it a new backend? (gitolite2, gitolite3, gitolite3_compile_1...) ?

cons: harder to reuse on src.fp.o w/o modifications, but that might be the case anyway

I have to say I don't have an opinion for this. What would we gain by making it a new backend? Is it the lack of new configuration key?

We gain separation of code and following the plugin system built in just for this (different gitolite backend update mechanism)

Your last comment on that PR said it was still WIP, what do you think is the current status?

And now that you saw pagure-dist-git, do you foresee much change there?

Ok, I understand that, but consider this:

  • A new backend means that everyone who has custom backend (like pagure-dist-git) will have to change their code to inherit from this new backend. If we use my proposed configuration keys, they can leave their code as-is and just change configuration.
  • My personal feeling is that there should be a difference between configuring a backend and writing a completely new backend. I still see this as an optimization of existing backend rather than something so radically different to constitute a new backend.
  • Another issue with doing this as a new backend is that it would mean reimplementing much of the generate_acls method in the new backend class (which I guess is not a huge issue, since we could pull this functionality out into individual methods and just call it from generate_acls)

All in all, I personally don't think this should be a new backend, but if you're going insist, I'll work on this :)

The only thing that's WIP about this PR now is the discussion whether or not to pull this into a new backend. I think pagure-dist-git should work absolutely fine, since it doesn't do anything non-standard, just adds some rules to individual repos.

Ok, thanks for your thoughts on the question. Let's keep it as is and we'll see in the future if we need to change it :)

Anything else you would like to do before we merge?

Nothing more to add here, ok to merge. Thanks!

rebased onto 577227fc66d40bb13f0d1aef09c33a919ebfd297

Hmm, give me some more time. Things suddenly started failing in one of my environments. It could be related to this patch, I want to find out what's going on before we merge.

Ok, I finally managed to track the source of issues in my staging environment and it was completely unrelated. This PR is good to be merged.

Let's do it! :)

rebased onto 6197adf40b672da8c2e3158b1aa3642cefbbfa90

Pull-Request has been merged by pingou

Metadata