PR#297: Make update work for containers - fedpkg

fedpkg

#297 Make update work for containers

Merged 5 years ago by lsedlar. Opened 5 years ago by lsedlar.

lsedlar/fedpkg container-update into master

Make update work for containers

Lubomír Sedlář • 5 years ago

bd55470

fedpkg/cli.py

file modified

+15 -6

		`@@ -644,8 +644,13 @@`
		`return False`

		`def _prepare_bodhi_template(self, template_file):`
		`+ try:`
		`+ nvr = self.cmd.nvr`
		`+ except rpkgError:`
		`+ # This is not an RPM, can't get NVR`
		`+ nvr = "FILL_IN_NVR_HERE"`
		`bodhi_args = {`
		`- 'nvr': self.cmd.nvr,`
		`+ 'nvr': nvr,`
		`'bugs': six.u(''),`
		`'descr': six.u(`
		`'Here is where you give an explanation of your update.'),`
		`@@ -662,10 +667,15 @@`
		`else:`
		`bodhi_args['type_'] = ''`

		`- self.cmd.clog()`
		`- clog_file = os.path.join(self.cmd.path, 'clog')`
		`- with io.open(clog_file, encoding='utf-8') as f:`
		`- clog = f.read()`
		`+ try:`
		`+ self.cmd.clog()`
		`+ clog_file = os.path.join(self.cmd.path, 'clog')`
		`+ with io.open(clog_file, encoding='utf-8') as f:`
		`+ clog = f.read()`
		`+ os.unlink(clog_file)`
		`+ except rpkgError:`
		`+ # Not an RPM, no changelog to work with`
		`+ clog = ""`

		`if self.args.bugs:`
		`bodhi_args['bugs'] = self.args.bugs`
		`@@ -721,7 +731,6 @@`
		`'as bodhi.template.last' % e)`
		`finally:`
		`os.unlink(bodhi_template_file)`
		`- os.unlink('clog')`
		`else:`
		`self.log.info('Bodhi update aborted!')`

lsedlar commented 5 years ago

We don't really have an NVR available, but users can fill it in by themselves.

Fixes: https://pagure.io/fedpkg/issue/296

rebased onto 3d606d24ad2ad7dda293071e5ef3b6ba228df895

5 years ago

rebased onto b0ee984e005ae22b1ffdac25b2db58d1227e976b

5 years ago

onosek commented on line 41 of fedpkg/cli.py 5 years ago

Is path missing?

lsedlar commented on line 41 of fedpkg/cli.py 5 years ago

Good catch, yes, this will break if --path is used.

rebased onto bd55470

5 years ago

onosek commented 5 years ago

That's fine for me. Thanks.

cqi commented 5 years ago

LGTM

cqi commented 5 years ago

Just an idea, would it be good to add a separate property or method to indicate current repo type. The code depending on repo type would look like:

if cmd.is_rpm_repo:
    ...
if cmd.is_container_repo:
    ...
if cmd.is_modules_repo:
    ...

Pull-Request has been merged by lsedlar

5 years ago

lsedlar commented 5 years ago

@cqi It's an option, but it seems like a lot of work when new type is added and it does not really prevent any problems as far as I can tell. For example fedpkg verrel is still broken for containers.

cqi commented 5 years ago

@lsedlar I agree. Current implementation is good enough.