Draft: GitLab events / parsing refactoring

[mfocko]

TODO:

• Write new tests or update the old ones to cover new functionality.
• Update doc-strings where appropriate.
• Update or write new documentation in packit/packit.dev.
• ‹fill in›

Fixes #2471

Related to

Merge before/after

RELEASE NOTES BEGIN

Packit now supports automatic ordering of ☕ after all checks pass.

RELEASE NOTES END

[softwarefactory-project-zuul]

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/b081b7e1f4494a828a916ebe8b9ad6c0

✔️ pre-commit SUCCESS in 2m 30s
✔️ packit-service-tests SUCCESS in 2m 44s

[majamassarini]

Thank you 🙏🏻
Looking at this I think I would like to have more namespaces in our code. I would like to have github.PullRequestEvent or even better event.github.PullRequest instead of GithubPullRequestEvent. I don't know what others think about it and I don't know if this could be the right moment for such a big refactoring... but still I think it would be nicer 🙂

Sorry I didn't realize it was still a draft.

[mfocko]

Thank you 🙏🏻 Looking at this I think I would like to have more namespaces in our code. I would like to have github.PullRequestEvent or even better event.github.PullRequest instead of GithubPullRequestEvent. I don't know what others think about it and I don't know if this could be the right moment for such a big refactoring... but still I think it would be nicer 🙂

I'm thinking about it too :D and I will need to do some changes around anyways, so… the biggest issue is fixing the imports afterwards :D

[mfocko]

I'm thinking about it too :D and I will need to do some changes around anyways, so… the biggest issue is fixing the imports afterwards :D

I think it will break everything, so… let's do it \o/

[mfocko]

How about…

packit_service.worker.events
|- abstract
|  |- AbstractForgeIndependentEvent
|  `- AbstractResultEvent
|- github
|  |- abstract
|  |  `- AbstractGithubEvent
|  |- pull_request
|  |  |- Comment
|  |  `- Update # ← not sure here, right now it's just PullRequestGithubEvent, covers create and push
…

@lbarcziova @majamassarini @packit/the-packit-team

The bigger issue how to fit the downstream events in there… Do they even count as downstream-only if they come from both GitLab and Pagure? It almost feels like there should be two parsers (parsing Fedora Messaging event) returning the same type (downstream event) regardless of whether it's from GitLab or Pagure. We shouldn't even care about this…

I don't want to add another layer of events.upstream.github.… (#матрёшка) :/

[lbarcziova]

It almost feels like there should be two parsers (parsing Fedora Messaging event) returning the same type (downstream event) regardless of whether it's from GitLab or Pagure.

you are right, this is probably how it should be done the most correctly

but otherwise the proposal looks good to me! Is it manageable to have it like that considering your note about upstream vs downstream? @mfocko

[mfocko]

you are right, this is probably how it should be done the most correctly

but otherwise the proposal looks good to me! Is it manageable to have it like that considering your note about upstream vs downstream? @mfocko

maybe

• packit_service.worker.events
  • copr
  • openscanhub
  • downstream
  • github
  • gitlab

?

I think that for the beginning I could just rename Pagure to downstream (hide the fact it's pagure) and try to make it more abstract for supporting GitLab (or potentially Forgejo too, if and when Fedora migrates)

[softwarefactory-project-zuul]

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/8f7b4b4f53054547b507e83d58bd9e3d

✔️ pre-commit SUCCESS in 2m 18s
✔️ packit-service-tests SUCCESS in 2m 31s

[mfocko]

GitHub PLEASE /o