Skip to content

Switch to using testing-farm multihost pipeline#3127

Draft
LecrisUT wants to merge 2 commits into
packit:mainfrom
LecrisUT:feat/testing-farm-multihost
Draft

Switch to using testing-farm multihost pipeline#3127
LecrisUT wants to merge 2 commits into
packit:mainfrom
LecrisUT:feat/testing-farm-multihost

Conversation

@LecrisUT
Copy link
Copy Markdown
Collaborator

TODO:

  • Write new tests or update the old ones to cover new functionality.
  • Make sure relevant plans are compatible

LecrisUT added 2 commits May 14, 2026 15:32
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds multihost support for Testing Farm by introducing a multihost parameter to the _get_tf_base_payload method, which configures the pipeline type as tmt-multihost. Several payload generation methods were updated to enable this flag. The reviewer requested an update to the _get_tf_base_payload docstring to include the new parameter and comply with the Google-style documentation requirements specified in the repository's style guide.

Comment on lines +1655 to +1660
def _get_tf_base_payload(
self,
distro: str,
compose: Optional[str],
multihost: bool = False,
) -> dict:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The docstring for _get_tf_base_payload should be updated to include the new multihost parameter and follow the Google-style format as required by the repository style guide (lines 412-414). It is currently missing the Args and Returns sections, which are necessary for non-trivial methods like this one that construct complex payload structures.

References
  1. All code must contain accurate and sufficiently detailed docstrings, formatted in accordance with the PEP 257 standard and in Google-style (including Args and Returns sections). (link)

@mfocko
Copy link
Copy Markdown
Member

mfocko commented May 14, 2026

Why do you want to turn it on by default? 👀

@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

@LecrisUT
Copy link
Copy Markdown
Collaborator Author

Why do you want to turn it on by default? 👀

Because if compose is not set, testing-farm will force it to be provision/container, which does not work for fedora-review. For all other cases with compose not set, mostly because there should not be any difference between these two. When compose is set things are tricky because these differ in the artifact installation which I would like to migrate these to using, but the implementation is just arriving today so still need to try it out.

It is a draft until can confirm that it is stable for those simple tests, will post an example run when I have it

@LecrisUT
Copy link
Copy Markdown
Collaborator Author

Yep, I've identified 2 bugs with multihost pipeline affecting these, so definitely not ready yet:

  • imported jobs automatically translate to provision/artemis even though it has provision/container. Will affect rpminspect migration to shared-tests
  • similarly provision/virtual is not translated to provision/artemis, affects fedora-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants