#332 Add --fail-fast functionality.
Merged 5 years ago by cqi. Opened 5 years ago by tibbs.
tibbs/rpkg fastfail  into  master

file modified
+8 -1
@@ -1891,7 +1891,8 @@ 

              commit_hash or self.commithash)

  

      def build(self, skip_tag=False, scratch=False, background=False,

-               url=None, chain=None, arches=None, sets=False, nvr_check=True):

+               fail_fast=False, url=None, chain=None, arches=None, sets=False,

I think, it is safer appending new parameters then inserting in the middle. I know, these parameters are not positional and hopefully nobody calls this method that way currently. I mean build(False, True, True, ...) So it is only minor suggestion.

+               nvr_check=True):

          """Initiate a build of the module.  Available options are:

  

          skip_tag: Skip the tag action after the build
@@ -1900,6 +1901,9 @@ 

  

          background: Perform the build with a low priority

  

+         fail_fast: Perform the build in fast failure mode, which will cause the

+         entire build to fail if any subtask/architecture build fails.

+ 

          url: A url to an uploaded srpm to build from

  

          chain: A chain build set
@@ -1962,6 +1966,9 @@ 

          if background:

              cmd.append('--background')

              priority = 5  # magic koji number :/

+         if fail_fast:

+             opts['fail_fast'] = True

+             cmd.append('--fail-fast')

          if arches:

              if not scratch:

                  raise rpkgError('Cannot override arches for non-scratch '

file modified
+12 -3
@@ -438,6 +438,9 @@ 

          self.build_parser_common.add_argument(

              '--background', action='store_true', default=False,

              help='Run the build at a low priority')

+         self.build_parser_common.add_argument(

+             '--fail-fast', action='store_true', default=False,

+             help='Fail the build immediately if any arch fails')

  

      def register_rpm_common(self):

          """Create a common parser for rpm commands"""
@@ -1402,9 +1405,15 @@ 

          nvr_check = True

          if hasattr(self.args, 'nvr_check'):

              nvr_check = self.args.nvr_check

-         task_id = self.cmd.build(self.args.skip_tag, self.args.scratch,

-                                  self.args.background, url, chain, arches,

-                                  sets, nvr_check)

+         task_id = self.cmd.build(skip_tag=self.args.skip_tag,

+                                  scratch=self.args.scratch,

+                                  background=self.args.background,

+                                  fail_fast=self.args.fail_fast,

+                                  url=url,

+                                  chain=chain,

+                                  arches=arches,

+                                  sets=sets,

+                                  nvr_check=nvr_check)

  

          # Log out of the koji session

          self.cmd.kojisession.logout()

This is sort of a minimal patch which hooks up koji's 'fail_fast' option, as I requested in #331. All it does is provide a --fail-fast flag, which can be used to turn on that functionality if the server is configured with the build_arch_can_fail option. I have applied this locally and tested against Fedora's koji and the fail_fast option does get propagated into the build and works as expected.

I didn't add any tests. The existing tests still pass, but as various options like --background or --scratch don't appear to be tested, I wasn't sure how to go about it. Given a bit of guidance, I can certainly try to add some tests.

The patch doesn't introduce any PEP8 failures; flake8 for me finds three pre-existing failures in pyrpkg/init.py.

I don't use bash so I didn't touch the bash completion file. I can try if that is necessary. If this is accepted I will provide a patch to the fedpkg zsh completion function.

Signed-off-by: Jason Tibbitts tibbs@math.uh.edu

Pretty please pagure-ci rebuild

Thank you very much. Looks good for me.

It is fine at this moment without tests on Commands.build. I'm working on another issue that needs to refactor that method and I plan to add tests for it.

It would be good to add this new option to bash completion in fedpkg.bash. It should work by adding it at https://pagure.io/fedpkg/blob/master/f/conf/bash-completion/fedpkg.bash#_101

It's appreciated to add this new option to zsh completion as well. Thank you.

Can you associate the commit with issue #331 in commit message? That would be helpful for us to manage the changelog. FYI, you can refer to previous commits to know the workable format.

I just barely figured out how to get a repo in format that would let me create a PR; the requirement for special signed-off commits is really annoying and I wasted a lot of time trying to figure out how to do it. (In the end I deleted my forked repo entirely and recreated it from scratch so that I could do git commit -s from the start.)

So I guess what I'm saying is that I have no idea how to change the commit message in the way you request. And I'll probably just attach patches to issues for any other changes moving forward because this project (and fedpkg) adds that extra barrier to sending PRs.

@tibbs

To sign-off the latest commit, git commit --amend -s.

To edit the commit message of the latest commit, git commit --amend.

@tibbs Can you update your PR and fix the conflicts as well? Thanks.

I think, it is safer appending new parameters then inserting in the middle. I know, these parameters are not positional and hopefully nobody calls this method that way currently. I mean build(False, True, True, ...) So it is only minor suggestion.

Commit 018a423 fixes this pull-request

Pull-Request has been merged by cqi

5 years ago