we should use msg['release_version'] instead of the msg['compose_id'] that will make the logic below simpler :-)

ogr has a get_file_content method (https://github.com/packit-service/ogr/blob/master/ogr/services/pagure/project.py#L352) maybe we should look at using this instead of the url.

might be good to have the code below in its own function ?

A few comments, we should also look at adding some unit tests :)

On second thought Could we just always use master, and have the information in the .toml file ? for example container.f32 = cverna container.f31=mboddu ? or something in that spirit ?

Is there an example of the maintainers.toml ?

On second thought Could we just always use master, and have the information in the .toml file ? for example container.f32 = cverna container.f31=mboddu ? or something in that spirit ?

This will become a huge list in couple of releases

Is there an example of the maintainers.toml ?

https://pagure.io/fork/mohanboddu/fedora-kickstarts/blob/maintainer-pings/f/maintainers.toml is what I used in my testings.

On second thought Could we just always use master, and have the information in the .toml file ? for example container.f32 = cverna container.f31=mboddu ? or something in that spirit ?

This will become a huge list in couple of releases

How often we have different maintainers for branches ?

Overall I was just thinking that it would be easier to have to maintain the maintainers.yaml only in one place and one branch. But I don't have a strong opinion on that :-), so happy to go with one per branches if you prefer it.

Is there an example of the maintainers.toml ?

https://pagure.io/fork/mohanboddu/fedora-kickstarts/blob/maintainer-pings/f/maintainers.toml is what I used in my testings.

On second thought Could we just always use master, and have the information in the .toml file ? for example container.f32 = cverna container.f31=mboddu ? or something in that spirit ?
This will become a huge list in couple of releases

How often we have different maintainers for branches ?

there is a bunch of epel7/8 packages not maintained by the fedora package maintainers.

On second thought Could we just always use master, and have the information in the .toml file ? for example container.f32 = cverna container.f31=mboddu ? or something in that spirit ?
This will become a huge list in couple of releases
How often we have different maintainers for branches ?

there is a bunch of epel7/8 packages not maintained by the fedora package maintainers.

Yeah, but isn't this in the context of the Spins ? Meaning that someone maintains XFCE in Fedora 31 and someone else maintains it in Fedora 30 ?

If that's in the context of packages then i misunderstood the change :-) and indeed that would make the file huge :-).

ogr has a get_file_content method (https://github.com/packit-service/ogr/blob/master/ogr/services/pagure/project.py#L352) maybe we should look at using this instead of the url.

I looked at this, but it returns the content as str which means, I need to load into toml anyway. And we are importing requests already, I am not sure how ogr will help in this case then. Maybe I am missing something, but I am happy to learn :)

This requires https://pagure.io/fedora-kickstarts/pull-request/618 to be merged.

Dont merge it yet, I am still working on some improvements.

@cverna Now its done.

Fixes #4

LGTM

you should be able to do
for variant in maintainers_info here instead of creating a variants list. In the for loop the dictionary will return its keys.

I think using regex might be easier here instead of going through the toml file. Something like
re.search(r"variant (\w+), arch (\S+), subvariant (\w+)", line) that would give 3 groups with the variant, arch and subvariant that you can then use to access the toml like
maintainers_info.get(subvariant, {}).get(arch, {}).get(subvariant, {}).get(fas)
I think that would simplify the code quite a bit, what do you think ?

It might be good to add one more maintainer there, so we test that case and also maybe one more spin or variant ?

Nice :)

I think using regex might be easier here instead of going through the toml file. Something like
re.search(r"variant (\w+), arch (\S+), subvariant (\w+)", line) that would give 3 groups with the variant, arch and subvariant that you can then use to access the toml like
maintainers_info.get(subvariant, {}).get(arch, {}).get(subvariant, {}).get(fas)
I think that would simplify the code quite a bit, what do you think ?

Thanks for the idea, I actually made better improvements.

I think we could use itertools here to make this line simpler, something like maintainers = set(itertools.chain(*maintainer_fas_list)). I would be good to add a comment also in that section to explain what it is doing :-).

nitpicking but returning a tuple would be better here, since tuple are immutable. You could also unpack the tuple like maintainers, variant, arch = get_maintainers(line, ks_url)

It would be better to have except Exception here. I don't think flake8 would be happy here.

if 'rawhide' == msg[...., you don't need the .lower()

I am not sure if this is needed ?

Should we handle stg ? If we keep it like that people will also get pinged on the tickets that are created against the staging instance, so we should probably
configure that url in the config file. What do you think ?

This is running on Python3.7 so we can use f-strings :fireworks:

same here we can use f-strings :-)

LGTM :thumbsup: