Yaniv Bronhaim has posted comments on this change. Change subject: Adding Vdsm cron job check for available packages update ......................................................................
Patch Set 10: (4 comments) https://gerrit.ovirt.org/#/c/38401/10/vdsm/Makefile.am File vdsm/Makefile.am: Line 73: dist_vdsm_SCRIPTS = \ Line 74: addNetwork \ Line 75: delNetwork \ Line 76: get-conf-item \ Line 77: updater-tool \ > the name is very generic; yet what it does is just poll for updates. The pr It planed to be separate to vdsm at all. some plans talk about using it in other services as well as external tool. I don't know if it will happen but still vdsm-tool verbs belong to vdsm related scripts\tools and this one is too general.. no? (I can find more general verbs like service, but still im not sure about that) Line 78: set-conf-item \ Line 79: supervdsmServer \ Line 80: vdsm \ Line 81: vdsm-restore-net-config \ https://gerrit.ovirt.org/#/c/38401/10/vdsm/updater-tool File vdsm/updater-tool: Line 84: ups = self.update() Line 85: for trns in ups: Line 86: if pkgs is None or trns.name in pkgs: Line 87: pkgInfo = {} Line 88: pkgInfo['fullname'] = "%s.%s %s-%s" % (trns.name, > why do we need to report duplicate info? we have name, we have version, why rpm reports that in this format as well so I assumed it could be relevant in this format. but you're right, I should split it to pkgInfo['arch'] and add the release to version. will do that. Line 89: trns.arch, Line 90: trns.version, Line 91: trns.release, Line 92: ) Line 144: 'will check all available updates') Line 145: ) Line 146: parser.add_argument('--provider', Line 147: default='yum', Line 148: choices=('yum', 'dnf'), > what are our plans regarding apt-get ? later .. ? we don't plan to add apt-get support for 3.6 but dnf we do Line 149: help=( Line 150: 'Specify package-management utility to use. \n' Line 151: 'Current available options - %s' % Line 152: ('yum, dnf (default: yum)')) Line 155: return parser.parse_args(sys.argv[1:]) Line 156: Line 157: Line 158: if __name__ == '__main__': Line 159: if os.geteuid() != 0: > don't you want to make it a vdsm-tool command? at least for proper placemen replied on that in reply to the Makefile comment Line 160: outputPrint('updater-tool must run as root') Line 161: sys.exit(1) Line 162: Line 163: args = _parse_args() -- To view, visit https://gerrit.ovirt.org/38401 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1f6b83e1c18c05142da095b0b537fa6d9aecb59e Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
