Skip to content


Subversion checkout URL

You can clone with
Download ZIP


No need to actually check message contents. #1

merged 1 commit into from

2 participants


@janeznemanic, how about this?

I got to thinking, and there isn't really a need to check the message contents against the pkgdb acls.

When we run /usr/local/bin/ once, it regenerates the local acls cache for every user and every package. So, if the acls don't match for one message or another, it doesn't actually matter for the script itself -- it will just go query pkgdb on its own to get all the correct information.

Furthermore, if we have queued up 16 messages while delaying action, we just want to run once, not 16 times (I think).


@janeznemanic could you provide review and comment?


About skipping the check of the message contents. I think you mentioned that requires some time to finish the job, so it is quite heavy workload. So if the consumer receives a lot of 'false' messages with wrong acls, without querying the pkgdb you would run for nothing. But I think that is more theoretical problem which probably doesn't happen in reality. On the other hand it is also true that queries of pkgdb take some time and require resources. So skipping the check of the message content is the right thing to do.
About running the just once instead of 16 times. In my original code you can find these lines in the last for loop:'/usr/local/bin/')
self.queued_messages = []

The idea behind these lines of code was to achieve exactly what you propose. But I didn't test my code so I don't know if it would work as was intended. I agree running the script once than 16 times is the right step.
Anyway this should get tested and deployed.


Cool. I agree! One thing I don't yet have figured out is this:

This code will run as a plugin to the fedmsg-hub daemon. The fedmsg-hub daemon everywhere runs as the user 'fedmsg'.

However, the genacls cronjob up to this point has been running as a local gen-acls user. I'm not sure of the best way to do this.

Perhaps, we grant the fedmsg user on pkgs01 the right to passwordless sudo to become the gen-acls user? We would then need to modify our to attempt to sudo.


@ralphbean I did some quick research about the problem and here is what I have found out. functions passes all the work to subprocess.Popen() class which enables you to control the creation of new a process more precisely. Here is the Popen's constructor from Python's documentation.

class subprocess.Popen(args, bufsize=0, executable=None, stdin=None, stdout=None, stderr=None, preexec_fn=None, close_fds=False, shell=False, cwd=None, env=None, universal_newlines=False, startupinfo=None, creationflags=0)

In our case the relevant parameters are args and preexec_fn. Preexec_fn parameter is a function that is called in new process before it is executed. So we need to change the code to something like this.

def change_subprocess_id(user_uid, user_gid):



So this is what I found out. I tested the above solution quickly and it seems to work. More detailed explanation can be found at:


Cool. @janeznemanic, how about I merge this pull request and we do a separate one for the gid/uid issue.

Would you like to implement it?

@ralphbean ralphbean merged commit c68ebe3 into develop
@ralphbean ralphbean deleted the feature/simplify branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 10, 2014
  1. @ralphbean
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 39 deletions.
  1. +9 −39
@@ -6,8 +6,6 @@
-import json
-import httplib
import pprint
import subprocess
@@ -53,40 +51,12 @@ def delayed_consume():
moksha.hub.reactor.reactor.callLater(self.delay, delayed_consume)
def action(self, messages):
-"Acting on %r" % pprint.pformat(messages))
- username = None
- for message in messages:
- msg = message['msg']
- if msg['status'] == 'Awaiting Review':
- continue
- # if username in message is different from the one in previous
- # message query the pkgdb for username's acls, else use the old
- # acls
- if msg['username'] != username:
- username = msg['username']
- connection = httplib.HTTPConnection("")
- getquery = '/api/packager/acl/{0}'.format(username)
- connection.request("GET", getquery)
- response = connection.getresponse().read()
- useracls = json.loads(response)
- # check if collection{branchname,name,version}, package{name}, acl,
- # status, username... match in pkgdb and fedmsg message, if there
- # is a match run genacls script
- for packageacl in useracls['acls']:
- if (
- msg['package_listing']['package']['name'] == packageacl['packagelist']['package']['name'] and
- msg['acl'] == packageacl['acl'] and
- msg['package_listing']['collection']['branchname'] == packageacl['packagelist']['collection']['branchname'] and
- msg['package_listing']['collection']['name'] == packageacl['packagelist']['collection']['name'] and
- msg['package_listing']['collection']['version'] == packageacl['packagelist']['collection']['version'] and
- msg['username'] == packageacl['fas_name']
- ):
- self.queued_messages = []
- return
- #genacls will update all acls so
- #clean messages queue and return
+ self.log.debug("Acting on %r" % pprint.pformat(messages))
+ command = '/usr/local/bin/'
+ return_code =
+ if return_code == 0:
+"%r successful" % command)
+ else:
+ self.log.error("%r exited with %r" % (command, return_code))
Something went wrong with that request. Please try again.