Should be using the base_install method from the BaseHook as most of the code from this method is redundant to it.
Check https://pagure.io/pagure/blob/master/f/pagure/hooks/pagure_ticket_hook.py#_105

Same as above but with the remove method

https://pagure.io/pagure/blob/master/f/pagure/hooks/pagure_ticket_hook.py#_118

In prod we likely only care about prod, not dev

What is this file?

Why was all this code removed? Or rather, apparently, put on a single line?

Thought about using .get() instead of a try/except?

I'm fairly sure I haven't touched this file :)

Needs to cascade on delete as well

What is this? the project for which the plugin is activated? Don't we already have this by .project.name?

I guess the plugin will work on the instance it's running at no? If so this value is in the configuration file, not need to store it in the DB

If that's the label displayed, do we really want to leave it up to the user?

That's a more serious question: Do we want pagure to support multiple jenkins or just one?

What is this one?

If we add a new dependency, we should mention it where we list all the dependencies

Why not using the method in pagure.lib itself?

Where is APP defined?

NM it's above, found it :)

That's already in pagure

We have a different way of doing things in pagure

Why these imports?

Where is this used?

BASE.metadata.bind That's not the way we do things in pagure

• You're in the UI, we do not put SQL queries in the UI

The logic here should allow you to access hook_token

If it's meant to be next to pagure, I'd rather have it in its own folder (cf the EV server or the milter...)

2014 ?

You could merge this 2 imports to 1 line

I don't think this is working :)

API token

No. that is the name of the project on Jenkins

What is someone have their own jenkins instance ?

Apparently I can't comment inline, so I'll put it here:

For the model fields

• pagure_name is indeed name of project on Pagure and probably not needed anymore

• The label displayed in the flag is stored in display_name. My reasoning for adding this was the option of having multiple integrations for a single project.

  What I imagined was that pull request against rpkg would trigger one job to run unit tests on rpkg and another that would run tests for fedpkg. Both would be reported to the same pull request.

  If you don't want to support this use-case, the code can be simplified a bit, because get_configs will return only one configuration.

• As for the before_request method, I'm sure there is a better way. Please note this code was the first time I worked with flask.

One thing that is missing now (and very easy to implement) is supporting pull requests against other branch than master. This is the general idea.

It would be awesome if you considered some future extensions: what is really missing in my prototype is some sort of check when Jenkins build finishes, but does not send a message. This actually happens quite a lot. My plan was to track started jobs and collect them in a cron job.

@lsedlar Greetings! I am improving things one at a time. And I could see your thought process , will try to fix things and improve as we evolve things. You have done an amazing job with PMCI, it saved me a lot of effort. I will keep on troubling you with my issues. :smile:

Can be simplified to : if not cfg:

Since we're inside pagure and have direct access to the DB, why doing a http request instead of just using pagure.lib?

from pagure import SESSION?

What is this?

I still don't fully understand why the changes here :)

Is moksha required or fedmsg?

And how do we check authorization ?

Since there is only one domain name pagure.io I doubt we want the link to include it :)

What's the use of this line?

Any reason for not using the same construct as we do elsewhere with SESSION?

You know you can directly access the configuration in the templates?

It has a trailing slash to avoid that i did the work around ! Did find and suitable jinja fuction!

Checking what the token is meant to be (cf the settings page), I don't think we want to be using this here

I am thinking on this design , should I just move the whole pagure_ci_flag function here ? I mean it is posting a flag to the pr

Or the other way around, move the comment, percent down into pagure_ci_flag and call it directly :)

Is the file pagure/pagureCI/__init__.py needed? I'm not seeing it being imported anywhere

Is the file pagure/pagureCI/init.py needed? I'm not seeing it being imported anywhere

Yes it is required since moksha takes consumer.py as a package and it considers the whole directory to be one package so its required for it to have __init__.py , if __init__.py is not there it will throw error I discussed it with Peter long ago.

Ok, thanks

Tell people how they can setup their user to have permission.
This has bitten me often in the rear when forgotten to do this.

Why plain text? Why not json like the rest of the API?

Unless I'm missing something, I'm missing any and all authentication of this request AT ALL.

How about a constant-time comparison?

Why not just: return ('Token mismatch', 401)?

Why is this 64 characters if you use uuid4, which is always 32 characters?

Alright, let's merge :)