PR#835: get_ami.py should now listen to fedimg.image.publish fedmsg - fedora-websites

fedora-websites

#835 get_ami.py should now listen to fedimg.image.publish fedmsg

Merged 5 years ago by robyduck. Opened 5 years ago by sinnykumari.

sinnykumari/fedora-websites master into master

get_ami.py should now listen to fedimg.image.publish fedmsg

Sinny Kumari • 5 years ago

dc3931a

tools/get_ami.py

file modified

+4 -8

		`@@ -20,7 +20,7 @@`
		`log = logging.getLogger()`

		`base_url = 'https://apps.fedoraproject.org/datagrepper/raw'`
		`- topic = "org.fedoraproject.prod.fedimg.image.upload"`
		`+ topic = "org.fedoraproject.prod.fedimg.image.publish"`


		`def get_page(page, pages, delta):`
		`@@ -34,8 +34,6 @@`
		`return response.json()`


		`- completed = lambda message: message['msg']['status'] == 'completed'`
		`-`

		`def get_messages(days):`
		`""" Generator that yields messages from datagrepper """`
		`@@ -45,8 +43,7 @@`
		`# Get the first page`
		`data = get_page(1, 'unknown', delta)`
		`for message in data['raw_messages']:`
		`- if completed(message):`
		`- yield message`
		`+ yield message`

		`more = functools.partial(get_page, pages=data['pages'], delta=delta)`

		`@@ -55,8 +52,7 @@`
		`data = more(page + 1)`

		`for message in data['raw_messages']:`
		`- if completed(message):`
		`- yield message`
		`+ yield message`


		`def parse_args():`
		`@@ -98,4 +94,4 @@`
		`extra.get('id', '').ljust(15),`
		`extra.get('virt_type', '').ljust(13),`
		`extra.get('vol_type', '').ljust(15),`
		`- )`
		`\ No newline at end of file`
		`+ )`

sinnykumari commented 5 years ago

With fedimg 2.0.0, org.fedoraproject.prod.fedimg.image.publish
fedmsg topic is used to notify that AMIs are successfully
published to AWS EC2 regions. Fedmsg topic
org.fedoraproject.prod.fedimg.image.upload is sent only when
image gets successfully uploaded to base region.

Signed-off-by: Sinny Kumari sinny@redhat.com

dustymabe commented 5 years ago

this looks good to me, but are there any other places that look for fedmsg within this repo that need to be updated?

also when should we merge this? do we need to wait for anything?

robyduck commented 5 years ago

Yeah, the get_amy.py is just a helper script for us, we use it to check if AMI IDs are detected correctly or to get prerelease AMIs without nagging other people for that.

There is also tools/fedimg_vars_lib.py, which listen to that, and it's used by getfedora.org and alt.fp.o to get AMI IDs. This is more important, so if you can add it to the PR I'm happy to merge it. I assume this change already is implemented in fedmsg now, otherwise we need to wait until it's working. No need to wait for any other milestones.

dustymabe commented 5 years ago

There is also tools/fedimg_vars_lib.py, which listen to that, and it's used by getfedora.org and alt.fp.o to get AMI IDs.

right. the tricky part here is we will have some images that have been released before 2.0.0 (i.e. fedora 28 cloud base images) and some images that have been released after 2.0.0 (the next two week atomic release). So for some period of time we'll have to support both (until fedora 29 most likely).

sinnykumari commented 5 years ago

As @robyduck mentioned this is an independent tool, so it won't really effect anything else.
It can be merged anytime.

Since, we are going to have Atomic Two Week Release today, I will send a separate PR which needs to be updated to get all AMIs list on Atomic download page.