#90 Write bugzilla overrides into pagure_poc.json
Merged 4 years ago by pingou. Opened 4 years 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
4 years ago

I've dropped that line

Metadata Update from @pingou:
- Request assigned

4 years 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 30311c6f417cdd84daf9f40fe1256290fd2eb41a

4 years ago

I've pushed the following change:

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

rebased onto b3e0ae133b14ed842228757f6657720aa3399149

4 years ago

the path mangling crept in again, removed...

To be tested but this looks fine at the moment.

rebased onto 9f90c52d88e5ba1d40f544f399dcab6e3c2c48cc

4 years 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 171d0c627e5985a64921972aa3e5b2869bf41157

4 years ago

rebased onto 50232e2258f8a25973989ae7a6f5d3784c8bb666

4 years 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 f63c2e6f07ae9fbf57ed7d8ba0251b0f4da6c1c2

4 years 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 0a1cd2f7c9b45e24f0e1ec54da5bed3b4deb9de0

4 years 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 f1d644b9ee4de7e53f3a3af6e849e8028d6a9bb5

4 years ago

rebased onto afb748ba2c3dfac8a205560f27c10e93255cac76

4 years 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 d632db0e80683840ad0a44325e0e482ba0e006e2

4 years 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
4 years 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 years ago

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

Pull-Request has been merged by pingou

4 years ago
Metadata