#90 Write bugzilla overrides into pagure_poc.json
Merged 4 months ago by pingou. Opened 7 months ago by karsten.
karsten/pagure-dist-git pagure_poc_assignees  into  master

file modified
+19 -6

@@ -18,6 +18,8 @@ 

  import pagure.lib.query

  from pagure.lib import model

  

+ import pagure_distgit.model as distgit

+ 

  if 'PAGURE_CONFIG' not in os.environ \

          and os.path.exists('/etc/pagure/pagure.cfg'):

      os.environ['PAGURE_CONFIG'] = '/etc/pagure/pagure.cfg'

@@ -31,7 +33,7 @@ 

      """ Creates a JSON blob containing the following structure:

      {

        namespace: {

-         package: main admin,

+         package: { main admin, epel_assignee, fedora_assignee },

          ...

        },

        ...

@@ -52,20 +54,31 @@ 

      session = pagure.lib.query.create_session(_config['DB_URL'])

  

      query = session.query(

-         model.Project.namespace, model.Project.name, model.User.user

+         model.Project.namespace, model.Project.name,

+         model.User.user, model.Project.id,

+         distgit.PagureBZOverride.epel_assignee,

+         distgit.PagureBZOverride.fedora_assignee

      ).filter(

          model.Project.user_id == model.User.id

      ).filter(

          model.Project.parent_id == None

      ).filter(

          model.Project.is_fork == False

-     )

+     ).outerjoin(distgit.PagureBZOverride)

  

      output = collections.defaultdict(dict)

- 

      for entry in query.all():

-         namespace, package, admin = entry

-         output[namespace][package] = admin

+         ( namespace, package,

+           admin, project_id,

+           epel_assignee, fedora_assignee,

+         ) = entry

+         epel_assignee = epel_assignee or admin

+         fedora_assignee = fedora_assignee or admin

+         output[namespace][package] = {

+             'admin': admin,

+             'epel': epel_assignee,

+             'fedora': fedora_assignee

+         }

  

      with open(os.path.join(args[0], 'pagure_poc.json'), 'w') as stream:

          json.dump(output, stream, indent=4, sort_keys=True)

While this is needed for testing, I don't think we want this committed :)

1 new commit added

  • drop sys.path mangling
7 months ago

I've dropped that line

Metadata Update from @pingou:
- Request assigned

7 months ago

Shouldn't there be a link between model.Project and distgit.PagureBZOverride in the WHERE clause of the query?

Can you elaborate on what you mean by that ?

Well, you are using two tables in one query and from what I can read, there are no constraint between these two tables in the WHERE clause of your query.

rebased onto 30311c6

7 months ago

I've pushed the following change:

  • ).filter(
  • model.Project.id == distgit.PagureBZOverride.project_id
    )

rebased onto b3e0ae1

7 months ago

the path mangling crept in again, removed...

To be tested but this looks fine at the moment.

rebased onto 9f90c52

7 months ago

Alright, so after local testing, this is not working, it only considered the project for which I have set an overrides, not the others, which makes sense considering the query we're running here.

that filter... I'll look into it

rebased onto 171d0c6

7 months ago

rebased onto 50232e2

7 months ago

Where is this being used?

So the current approach leads to results such as:

"criu": {
    "admin": "pingou",
    "epel": null,
    "fedora": "ralph"
},

Where epel is wrong, it should be pingou not null.

Also a better approach to this would be to do a join between the two tables, I guess an outer join (no certainty though, I always have to check which type of join is needed), which allow to do everything in one query as you had previously tried.

an outer join (left for example) + coalesce to select owner if assignee is null?

rebased onto f63c2e6

7 months ago

I need to read up on outer joints as that might be much faster. For now I've fixed the code to use the repo admin when the assignee is unset

You explode entry into multiple variables above and here you're going back to it :(

I'm pretty sure flake8 wants that line

I think I've already asked it: where is this used?

I think I've already asked it: where is this used?

At the very bottom, but there's another import sys. This line can and will go

rebased onto 0a1cd2f

7 months ago

You were right, flake wanted that empty line, along with some minor other fixes.

Did you try running the script?

I find it quite amusing that when I try to run your PR as you push it I keep running into:

AttributeError: module 'pagure_distgit.model' has no attribute 'PagureBZOverride'

Are you even running the script before you push your changes?

rebased onto f1d644b

7 months ago

rebased onto afb748b

7 months ago

Yes, I run the script in vagrant and it succeeds:
(python3-pagure) [vagrant@pagure-dev pagure-dist-git]$ PAGURE_CONFIG=/home/vagrant/pagure.cfg PYTHONPATH=.:/home/vagrant/devel/ python3 scripts/pagure_poc.py /tmp/scriptoutput/

After that command there's a /tmp/scriptoutput/pagure_poc.json and its contents look ok

You need to run it with the other patch from https://pagure.io/pagure-dist-git/pull-request/82 in place, otherwise you'll see the "no attribute 'PagureBZOverride'" error

#82 is merged, so could you rebase this pr to current master? Thanks :)

rebased onto d632db0

7 months ago

that's probably what Pierre meant with his comment, I just didn't get it ;-(
Rebased...

What's the status of this ?

I'm looking at outer joints docs atm

1 new commit added

  • use left outerjoin
6 months ago

I've also tried coalesce as suggested, but that fails when one of the assignees is set to an empty string instead of None. The current version works for both, '' and None. I don't think the current pagure-dist-git version will write '' into the db anymore, but this seems to be the save way to handle this special case.

rebased onto 49a76ce

4 months ago

Let's get this in, thanks for your work on all this! :)

Pull-Request has been merged by pingou

4 months ago
Metadata