will it make sense to use either of _log, log or LOGGER everywhere ? I mean same var name

this is meant to be here?

+1 but these are currently different services, so I'd suggest we do that in another PR

it's not meant to stay there at least, I'll remove it, thanks for catching it!

the comment here, has been corrupted.

something is wrong here ! os.path.gure ?

will fix

Good catch

any reason for not using config ?

Is this wanted ?

It has been moved: pagure/ui/__init__.py:@UI_NS.route('/api/')

I think it was just easier and if we need at one point to move some things again we can just re-adjust the import w/o (hopefully) changing more things

It would be nice to add some doc string here

Let's be consistent with other docs string and use """

maybe we could store the FROM_EMAIL in a global variable and resuse this variable here and after in the module, instead of getting it from the config everytime.

Why do we need to pass the DB_URL now ?

this is duplicated ?

missing ui_ns here before .add_group ?

same here missing ui_ns ?

but that would no longer work if the config is changed after the module is loaded, say the default changes for a hook or a CLI tool or something similar.

because before a session was created upon importing the module and thus the sessionmaker was instanciated, thus allowing us to get a new session using the sessionmaker w/o passing the db_url :)

apparently :)

added :)

added

Something wrong here :)

Did this changed ?

was for testing, will change it back, thanks for catching it!

I'll double check, thanks

so I think the original test was broken, since the first 3 characters are removed above and should not be added back anywhere

os.path.join here has only one argument? https://pagure.io/pagure/blob/rearch/f/pagure/config.py#_15

_config = pagure.config.config was good enough but ok

why not move get_proejct_users to utils.py as well?

No I really wanted to force-reload the config :)

It's in the model I don't think we need to change this. Why do you think we should?

os.path.join here has only one argument? https://pagure.io/pagure/blob/rearch/f/pagure/config.py#_15

Looks like you're right and it can be dropped, thanks for finding this one :)

Reviewed the best I could :eyeglasses: .

Looks good to me +1

:thumbsup: