Freeze break request: pkgdb_sync should only remove master from todo if it was requested
Hi,
So another freeze break request, +1s appreciated. This change is the only thing needed to make non-master-repos an option. Currently, the code always removes master from the requested branches, and crashes if it wasn't there. I'm just adding a check to only remove it (and announce the package) if it was requested.
commit d230b5701575879e757f2c4aeb8eed62c319d9e6 Author: Patrick Uiterwijk puiterwijk@redhat.com Date: Wed Feb 25 11:34:07 2015 +0000
Only remove master from request if it was requested
Signed-off-by: Patrick Uiterwijk puiterwijk@redhat.com
diff --git a/roles/distgit/templates/pkgdb_sync_git_branches.py b/roles/distgit/templates/pkgdb_sync_git_branches.py index c931d26..ced139b 100644 --- a/roles/distgit/templates/pkgdb_sync_git_branches.py +++ b/roles/distgit/templates/pkgdb_sync_git_branches.py @@ -189,16 +189,17 @@ def branch_package(pkgname, branches): if not os.path.exists( os.path.join(GIT_FOLDER, '%s.git' % pkgname)): _invoke(SETUP_PACKAGE, [pkgname]) - branches.remove('master') # SETUP_PACKAGE creates master - fedmsg.publish( - topic='branch', - modname='git', - msg=dict( - agent='pkgdb', - name=pkgname, - branch='master', - ), - ) + if 'master' in branches: + branches.remove('master') # SETUP_PACKAGE creates master + fedmsg.publish( + topic='branch', + modname='git', + msg=dict( + agent='pkgdb', + name=pkgname, + branch='master', + ), + )
# Create all the required branches for the package # Use the translated branch name until pkgdb falls inline
With kind regards, Patrick Uiterwijk Fedora Infra
On Wed, Feb 25, 2015 at 06:36:27AM -0500, Patrick Uiterwijk wrote:
Hi,
So another freeze break request, +1s appreciated. This change is the only thing needed to make non-master-repos an option. Currently, the code always removes master from the requested branches, and crashes if it wasn't there. I'm just adding a check to only remove it (and announce the package) if it was requested.
commit d230b5701575879e757f2c4aeb8eed62c319d9e6 Author: Patrick Uiterwijk puiterwijk@redhat.com Date: Wed Feb 25 11:34:07 2015 +0000
Only remove master from request if it was requested Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>diff --git a/roles/distgit/templates/pkgdb_sync_git_branches.py b/roles/distgit/templates/pkgdb_sync_git_branches.py index c931d26..ced139b 100644 --- a/roles/distgit/templates/pkgdb_sync_git_branches.py +++ b/roles/distgit/templates/pkgdb_sync_git_branches.py @@ -189,16 +189,17 @@ def branch_package(pkgname, branches): if not os.path.exists( os.path.join(GIT_FOLDER, '%s.git' % pkgname)): _invoke(SETUP_PACKAGE, [pkgname])
branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)
if 'master' in branches:branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)
Could you expand a little about this fedmsg message? We don't create the branch (created by SETUP_PACKAGE) but we still send a message that we did?
Pierre
----- Original Message -----
On Wed, Feb 25, 2015 at 06:36:27AM -0500, Patrick Uiterwijk wrote:
Could you expand a little about this fedmsg message? We don't create the branch (created by SETUP_PACKAGE) but we still send a message that we did?
That's because the SETUP_PACKAGE script doesn't fire off fedmsg messages. As far as I know, there's no fedmsg message for new-repo.
Pierre _______________________________________________
On Wed, Feb 25, 2015 at 08:50:25AM -0500, Patrick Uiterwijk wrote:
----- Original Message -----
On Wed, Feb 25, 2015 at 06:36:27AM -0500, Patrick Uiterwijk wrote:
Could you expand a little about this fedmsg message? We don't create the branch (created by SETUP_PACKAGE) but we still send a message that we did?
That's because the SETUP_PACKAGE script doesn't fire off fedmsg messages. As far as I know, there's no fedmsg message for new-repo.
But we could have it fire that same message about the master branch being added, no? (I'm asking if we could move that same piece of code to the place where the master branch is being created)
Pierre
----- Original Message -----
On Wed, Feb 25, 2015 at 08:50:25AM -0500, Patrick Uiterwijk wrote:
But we could have it fire that same message about the master branch being added, no? (I'm asking if we could move that same piece of code to the place where the master branch is being created)
If we would do that, we would need to also add fedmsg initialization to the setup_git_package script. While this probably can be done, I think it's most clear to have the python script (pkgdb_sync) initialize it just once and sent the messages there. Otherwise we'd likely end up with a bunch of json etc in the scripts.
Pierre
Patrick
On Wed, Feb 25, 2015 at 08:56:40AM -0500, Patrick Uiterwijk wrote:
----- Original Message -----
On Wed, Feb 25, 2015 at 08:50:25AM -0500, Patrick Uiterwijk wrote:
But we could have it fire that same message about the master branch being added, no? (I'm asking if we could move that same piece of code to the place where the master branch is being created)
If we would do that, we would need to also add fedmsg initialization to the setup_git_package script. While this probably can be done, I think it's most clear to have the python script (pkgdb_sync) initialize it just once and sent the messages there. Otherwise we'd likely end up with a bunch of json etc in the scripts.
It's shell script, so we could just relay on calling fedmsg-logger, the arguments are just the agent, the name of the package and the name of the branch (ie master), 2 of the 3 are hard-coded and we know the third one (the package name).
Might not be that much json at the end :)
Maybe something along the lines of:
fedmsg-logger --message='{"agent": "pkgdb", "name": "$PACKAGE", "branch": "master"}' \ --json-input --modname=git --topic=branch
I guess we may want to ask Ralph for some input here
But then again, that's something for after the freeze :)
Pierre
On Wed, Feb 25, 2015 at 01:48:12PM +0100, Pierre-Yves Chibon wrote:
On Wed, Feb 25, 2015 at 06:36:27AM -0500, Patrick Uiterwijk wrote:
Hi,
So another freeze break request, +1s appreciated. This change is the only thing needed to make non-master-repos an option. Currently, the code always removes master from the requested branches, and crashes if it wasn't there. I'm just adding a check to only remove it (and announce the package) if it was requested.
commit d230b5701575879e757f2c4aeb8eed62c319d9e6 Author: Patrick Uiterwijk puiterwijk@redhat.com Date: Wed Feb 25 11:34:07 2015 +0000
Only remove master from request if it was requested Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>diff --git a/roles/distgit/templates/pkgdb_sync_git_branches.py b/roles/distgit/templates/pkgdb_sync_git_branches.py index c931d26..ced139b 100644 --- a/roles/distgit/templates/pkgdb_sync_git_branches.py +++ b/roles/distgit/templates/pkgdb_sync_git_branches.py @@ -189,16 +189,17 @@ def branch_package(pkgname, branches): if not os.path.exists( os.path.join(GIT_FOLDER, '%s.git' % pkgname)): _invoke(SETUP_PACKAGE, [pkgname])
branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)
if 'master' in branches:branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)
+1 for the change as it fixes the 'bug' we saw and doesn't change from the current situation. But sending the message at a different place than where the action is done doesn't sound right for me, maybe something for later.
Pierre
On Wed, 25 Feb 2015 06:36:27 -0500 (EST) Patrick Uiterwijk puiterwijk@redhat.com wrote:
Hi,
So another freeze break request, +1s appreciated. This change is the only thing needed to make non-master-repos an option.
I'm pretty sure we don't want to decide to change this option during freeze. ;)
Currently, the code always removes master from the requested branches, and crashes if it wasn't there. I'm just adding a check to only remove it (and announce the package) if it was requested.
I guess +1 to this anyhow, as it could fix a error condition.
kevin --
commit d230b5701575879e757f2c4aeb8eed62c319d9e6 Author: Patrick Uiterwijk puiterwijk@redhat.com Date: Wed Feb 25 11:34:07 2015 +0000
Only remove master from request if it was requested Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>diff --git a/roles/distgit/templates/pkgdb_sync_git_branches.py b/roles/distgit/templates/pkgdb_sync_git_branches.py index c931d26..ced139b 100644 --- a/roles/distgit/templates/pkgdb_sync_git_branches.py +++ b/roles/distgit/templates/pkgdb_sync_git_branches.py @@ -189,16 +189,17 @@ def branch_package(pkgname, branches): if not os.path.exists( os.path.join(GIT_FOLDER, '%s.git' % pkgname)): _invoke(SETUP_PACKAGE, [pkgname])
branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)
if 'master' in branches:branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)# Create all the required branches for the package # Use the translated branch name until pkgdb falls inline
With kind regards, Patrick Uiterwijk Fedora Infra
infrastructure mailing list infrastructure@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/infrastructure
Thanks. And I personally think we should not do this, but was told we should consider the option.
With kind regards, Patrick Uiterwijk Fedora Infra
----- Original Message -----
On Wed, 25 Feb 2015 06:36:27 -0500 (EST) Patrick Uiterwijk puiterwijk@redhat.com wrote:
Hi,
So another freeze break request, +1s appreciated. This change is the only thing needed to make non-master-repos an option.
I'm pretty sure we don't want to decide to change this option during freeze. ;)
Currently, the code always removes master from the requested branches, and crashes if it wasn't there. I'm just adding a check to only remove it (and announce the package) if it was requested.
I guess +1 to this anyhow, as it could fix a error condition.
kevin
commit d230b5701575879e757f2c4aeb8eed62c319d9e6 Author: Patrick Uiterwijk puiterwijk@redhat.com Date: Wed Feb 25 11:34:07 2015 +0000
Only remove master from request if it was requested Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>diff --git a/roles/distgit/templates/pkgdb_sync_git_branches.py b/roles/distgit/templates/pkgdb_sync_git_branches.py index c931d26..ced139b 100644 --- a/roles/distgit/templates/pkgdb_sync_git_branches.py +++ b/roles/distgit/templates/pkgdb_sync_git_branches.py @@ -189,16 +189,17 @@ def branch_package(pkgname, branches): if not os.path.exists( os.path.join(GIT_FOLDER, '%s.git' % pkgname)): _invoke(SETUP_PACKAGE, [pkgname])
branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)
if 'master' in branches:branches.remove('master') # SETUP_PACKAGE creates masterfedmsg.publish(topic='branch',modname='git',msg=dict(agent='pkgdb',name=pkgname,branch='master',),)# Create all the required branches for the package # Use the translated branch name until pkgdb falls inline
With kind regards, Patrick Uiterwijk Fedora Infra
infrastructure mailing list infrastructure@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/infrastructure
infrastructure mailing list infrastructure@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/infrastructure
infrastructure@lists.fedoraproject.org
-
Kevin Fenzi -
Patrick Uiterwijk -
Pierre-Yves Chibon