Needs #2362 and #2352 merged before

The Jenkins tests seem to be unable to get connections to the database.

I recommend documenting the type of backend here.

It would be good to add a docblock to this class.

It would be good to add a docblock to this method.

Would this ever be used on large files? It could cause memory issues since it loads the whole file into memory at once.

I recommend documenting each of the parameters and their types.

This method is pretty long and a bit hard to follow since it uses many levels of indentation at points. I recommend breaking it up into smaller helper functions. That should also make it easier to test.

I don't see cls being used below - you could convert this to a @staticmethod.

It might be important to check on the exit code of this process. If it fails that could lead to unexpected behaviors in Pagure, but without exit code checking and logging, it won't be obvious that this cmd was the problem.

You can also make this one a @staticmethod.

This could also be static, and it would be nice to add a docblock.

Maybe drop this print statement.

LGTM

okido

Note that it looks like you reviewed mainly https://pagure.io/pagure/pull-request/2352 :)

Done

Done

It's the test backend, so all it's doing is this print statement :)

Done

Good idea

Done

Done

Done

It may but I expect these files will be fairly small. The method is used to read the files to include at the top/bottom of the gitolite config file.

And note that these files are configured in the configuration file of pagure, not by the users

Turns out if I do I break the inheritance since this method is called in generate_acls and overriden in Gitolite3Auth

Scratch that I think that was me

looks good :bikini:

Thanks everyone! :)