PR#837: Listen to fedimg.image.publish topic to get latest Atomic AMI list - fedora-websites

fedora-websites

#837 Listen to fedimg.image.publish topic to get latest Atomic AMI list

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

sinnykumari/fedora-websites atomic_ami_fix into master

Listen to fedimg.image.publish topic to get latest Atomic AMI list

Sinny Kumari • 5 years ago

2b1715b

tools/fedimg_vars_lib.py

file modified

+2 -2

		`@@ -26,7 +26,7 @@`
		`cache_file = '/tmp/fedora_websites_fedimg.cache'`
		`dateformat = '%Y-%m-%dT%H:%MZ'`
		`base_url = 'https://apps.fedoraproject.org/datagrepper/raw'`
		`- topic = "org.fedoraproject.prod.fedimg.image.upload"`
		`+ topic = "org.fedoraproject.prod.fedimg.image.publish"`

		`session = requests.session()`

		`@@ -62,7 +62,7 @@`

		`def filter_messages(messages, target):`
		`for message in messages:`
		`- if target in str(message) and 'completed' in str(message):`
		`+ if target in str(message):`
		`yield message`

sinnykumari commented 5 years ago

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

sinnykumari commented 5 years ago

This can be merged without any impact on current Atomic page on Website. This is because fedimg is publishing fedimg.image.publish from past 1 months. So, existing website will work without any issue.

Tested locally.

robyduck commented 5 years ago

Ok, but will this work also in a week or two? As @dustymabe mentioned, Cloud Base AMIs use a different version than Atomic AMIs. If you are sure it will not break anything in the next weeks, I'm fine with this PR. +1
Thank you.

dustymabe commented 5 years ago

@sinnykumari so you're saying the fedora cloud base AMIs also had a fedimg.image.publish fedmsg ?

dustymabe commented 5 years ago

basically if the answer to the following question is "YES" then I'm +1:

Will the Fedora Cloud Base AMIs from F28 release day still show up on the website?

Also, please rebase so we can prevent merge commit

sinnykumari commented 5 years ago

Yes, it will work for Cloud Base AMIs too. Both Cloud Base and Atomic Host AMIs are published using same fedmig version.

A bit of context:
fedimg-1.2.0 was deployed around 1 months back. In this release, fedimg was sending fedimg.image.upload for all regions (for backward compatibility) and also fedimg.image.publish (which is newly added). In fedimg-2.0.0, we use fedimg.image.upload only for successful upload notification and fedimg.image.publish when AMIs successfully gets published to regions.
Fedimg releases - https://github.com/fedora-infra/fedimg/releases

That's why we have fedimg.image.publish fedmsg for both Atomic and Cloud Base AMIs from past 1 months. This includes fedmsg for AMIs published during F28 GA for Cloud Base which was from 25th April.

This PR is to make sure that AMIs get listed properly for future releases. For example: without this PR, Atomic page on website won't show AMis list.

I have rebuilt locally alt.f.p.o as well with this change and it shows Fedora Cloud Base AMIs correctly.

rebased onto 2b1715b

5 years ago

robyduck commented 5 years ago

Ok then let's go for it.