GNOME.org
 

[Notes] [Git][BuildGrid/buildgrid][finn/74-operation-cancelation] 17 commits: Update CONTRIBUTING guide, add COMMITTERS list

Title: GitLab

finn pushed to branch finn/74-operation-cancelation at BuildGrid / buildgrid

Commits:

18 changed files:

Changes:

  • COMMITTERS.md
    1 
    +## COMMITTERS
    2 
    +
    3 
    +| Name | Email |
    4 
    +| -------- | -------- |
    5 
    +|  Carter Sande  | <carter.sande@duodecima.technology> |
    6 
    +|  Ed Baunton  | <edbaunton gmail com> |
    7 
    +|  Laurence Urhegyi  | <laurence urhegyi codethink co uk> |
    8 
    +|  Finn Ball  | <finn ball codethink co uk> |
    9 
    +|  Paul Sherwood  | <paul sherwood codethink co uk> |
    10 
    +|  James Ennis  | <james ennis codethink com> |
    11 
    +|  Jim MacArthur  | <jim macarthur codethink co uk> |
    12 
    +|  Juerg Billeter  | <juerg billeter codethink co uk> |
    13 
    +|  Martin Blanchard  | <martin blanchard codethink co uk> |
    14 
    +|  Marios Hadjimichael  | <mhadjimichae bloomberg net> |
    15 
    +|  Raoul Hidalgo Charman  | <raoul hidalgocharman codethink co uk> |
    16 
    +|  Rohit Kothur  |  <rkothur bloomberg net> |

  • CONTRIBUTING.rst
    ... ... @@ -32,40 +32,31 @@ side effects and quirks the feature may have introduced. More on this below in
    32 32
    33 33 
     .. _BuildGrid mailing list: https://lists.buildgrid.build/cgi-bin/mailman/listinfo/buildgrid
    34 34
    35 
    -
    36 35 
     .. _patch-submissions:
    37 36
    38 37 
     Patch submissions
    39 38 
     -----------------
    40 39
    41 
    -We are running `trunk based development`_. The idea behind this is that merge
    42 
    -requests to the trunk will be small and made often, thus making the review and
    43 
    -merge process as fast as possible. We do not want to end up with a huge backlog
    44 
    -of outstanding merge requests. If possible, it is preferred that merge requests
    45 
    -address specific points and clearly outline what problem they are solving.
    46 
    -
    47 
    -Branches must be submitted as merge requests (MR) on GitLab and should be
    48 
    -associated with an issue, whenever possible. If it's a small change, we'll
    49 
    -accept an MR without it being associated to an issue, but generally we prefer an
    50 
    -issue to be raised in advance. This is so that we can track the work that is
    40 
    +Branches must be submitted as merge requests (MR) on GitLab and should have a
    41 
    +corresponding issue raised in advance (whenever possible). If it's a small change,
    42 
    +an MR without it being associated to an issue is okay, but generally we prefer an
    43 
    +issue to be raised in advance so that we can track the work that is
    51 44 
     currently in progress on the project.
    52 45
    46 
    +When submitting a merge request, please obtain a review from another committer
    47 
    +who is familiar with the area of the code base which the branch effects. An
    48 
    +approval from another committer who is not the patch author will be needed
    49 
    +before any merge (we use Gitlab's 'approval' feature for this).
    50 
    +
    53 51 
     Below is a list of good patch submission good practice:
    54 52
    55 53 
     - Each commit should address a specific issue number in the commit message. This
    56 54 
       is really important for provenance reasons.
    57 
    -- Merge requests that are not yet ready for review must be prefixed with the
    58 
    -  ``WIP:`` identifier, but if we stick to trunk based development then the
    59 
    -  ``WIP:`` identifier will not stay around for very long on a merge request.
    60 
    -- When a merge request is ready for review, please find someone willing to do
    61 
    -  the review (ideally a maintainer) and assign them the MR, leaving a comment
    62 
    -  asking for their review.
    55 
    +- Merge requests that are not yet ready for review should be prefixed with the
    56 
    +  ``WIP:`` identifier.
    63 57 
     - Submitted branches should not contain a history of work done.
    64 58 
     - Unit tests should be a separate commit.
    65 59
    66 
    -.. _trunk based development: https://trunkbaseddevelopment.com
    67 
    -
    68 
    -
    69 60 
     Commit messages
    70 61 
     ~~~~~~~~~~~~~~~
    71 62
    ... ... @@ -89,6 +80,57 @@ For more tips, please read `The seven rules of a great Git commit message`_.
    89 80
    90 81 
     .. _The seven rules of a great Git commit message: https://chris.beams.io/posts/git-commit/#seven-rules
    91 82
    83 
    +.. _committer-access:
    84 
    +
    85 
    +Committer access
    86 
    +----------------
    87 
    +
    88 
    +Committers in the BuildGrid project are those folks to whom the right to
    89 
    +directly commit changes to our version controlled resources has been granted.
    90 
    +While every contribution is
    91 
    +valued regardless of its source, not every person who contributes code to the
    92 
    +project will earn commit access. The `COMMITTERS`_ file lists all committers.
    93 
    +
    94 
    +.. _COMMITTERS: https://gitlab.com/BuildGrid/buildgrid/blob/master/COMMITTERS.md
    95 
    +.. _Subversion: http://subversion.apache.org/docs/community-guide/roles.html#committers
    96 
    +
    97 
    +
    98 
    +How commit access is granted
    99 
    +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    100 
    +
    101 
    +After someone has successfully contributed a few non-trivial patches, some full
    102 
    +committer, usually whoever has reviewed and applied the most patches from that
    103 
    +contributor, proposes them for commit access. This proposal is sent only to the
    104 
    +other full committers -- the ensuing discussion is private, so that everyone can
    105 
    +feel comfortable speaking their minds. Assuming there are no objections, the
    106 
    +contributor is granted commit access. The decision is made by consensus; there
    107 
    +are no formal rules governing the procedure, though generally if someone strongly
    108 
    +objects the access is not offered, or is offered on a provisional basis.
    109 
    +
    110 
    +This of course relies on contributors being responsive and showing willingness
    111 
    +to address any problems that may arise after landing patches. However, the primary
    112 
    +criterion for commit access is good judgment.
    113 
    +
    114 
    +You do not have to be a technical wizard, or demonstrate deep knowledge of the
    115 
    +entire codebase to become a committer. You just need to know what you don't
    116 
    +know. If your patches adhere to the guidelines in this file, adhere to all the usual
    117 
    +unquantifiable rules of coding (code should be readable, robust, maintainable, etc.),
    118 
    +and respect the Hippocratic Principle of "first, do no harm", then you will probably
    119 
    +get commit access pretty quickly. The size, complexity, and quantity of your patches
    120 
    +do not matter as much as the degree of care you show in avoiding bugs and minimizing
    121 
    +unnecessary impact on the rest of the code. Many full committers are people who have
    122 
    +not made major code contributions, but rather lots of small, clean fixes, each of
    123 
    +which was an unambiguous improvement to the code. (Of course, this does not mean the
    124 
    +project needs a bunch of very trivial patches whose only purpose is to gain commit
    125 
    +access; knowing what's worth a patch post and what's not is part of showing good
    126 
    +judgement.)
    127 
    +
    128 
    +When submitting a merge request, please obtain a review from another committer
    129 
    +who is familiar with the area of the code base which the branch effects. Asking on
    130 
    +slack is probably the best way to go about this. An approval from a committer
    131 
    +who is not the patch author will be needed before any merge (we use Gitlab's
    132 
    +'approval' feature for this).
    133 
    +
    92 134
    93 135 
     .. _coding-style:
    94 136
    ... ... @@ -198,35 +240,6 @@ trunk.
    198 240
    199 241 
     .. _coverage report: https://buildgrid.gitlab.io/buildgrid/coverage/
    200 242
    201 
    -
    202 
    -.. _committer-access:
    203 
    -
    204 
    -Committer access
    205 
    -----------------
    206 
    -
    207 
    -We'll hand out commit access to anyone who has successfully landed a single
    208 
    -patch to the code base. Please request this via Slack or the mailing list.
    209 
    -
    210 
    -This of course relies on contributors being responsive and showing willingness
    211 
    -to address any problems that may arise after landing branches.
    212 
    -
    213 
    -When submitting a merge request, please obtain a review from another committer
    214 
    -who is familiar with the area of the code base which the branch effects. An
    215 
    -approval from another committer who is not the patch author will be needed
    216 
    -before any merge (we use gitlab's 'approval' feature for this).
    217 
    -
    218 
    -What we are expecting of committers here in general is basically to escalate the
    219 
    -review in cases of uncertainty.
    220 
    -
    221 
    -.. note::
    222 
    -
    223 
    -   We don't have any detailed policy for "bad actors", but will of course handle
    224 
    -   things on a case by case basis - commit access should not result in commit
    225 
    -   wars or be used as a tool to subvert the project when disagreements arise.
    226 
    -   Such incidents (if any) would surely lead to temporary suspension of commit
    227 
    -   rights.
    228 
    -
    229 
    -
    230 243 
     .. _gitlab-features:
    231 244
    232 245 
     GitLab features

  • MAINTAINERS deleted
    1 
    -Finn Ball
    2 
    -E-mail: finn ball codethink co uk
    3 
    -Userid: finnball

  • buildgrid/_app/bots/buildbox.py
    ... ... @@ -13,6 +13,7 @@
    13 13 
     # limitations under the License.
    14 14
    15 15
    16 
    +import logging
    16 17 
     import os
    17 18 
     import subprocess
    18 19 
     import tempfile
    ... ... @@ -29,7 +30,8 @@ def work_buildbox(context, lease):
    29 30 
         """
    30 31 
         local_cas_directory = context.local_cas
    31 32 
         # instance_name = context.parent
    32 
    -    logger = context.logger
    33 
    +
    34 
    +    logger = logging.getLogger(__name__)
    33 35
    34 36 
         action_digest = remote_execution_pb2.Digest()
    35 37

  • buildgrid/_app/bots/host.py
    ... ... @@ -13,6 +13,7 @@
    13 13 
     # limitations under the License.
    14 14
    15 15
    16 
    +import logging
    16 17 
     import os
    17 18 
     import subprocess
    18 19 
     import tempfile
    ... ... @@ -26,7 +27,7 @@ def work_host_tools(context, lease):
    26 27 
         """Executes a lease for a build action, using host tools.
    27 28 
         """
    28 29 
         instance_name = context.parent
    29 
    -    logger = context.logger
    30 
    +    logger = logging.getLogger(__name__)
    30 31
    31 32 
         action_digest = remote_execution_pb2.Digest()
    32 33 
         action_result = remote_execution_pb2.ActionResult()

  • buildgrid/_app/commands/cmd_bot.py
    ... ... @@ -20,7 +20,6 @@ Bot command
    20 20 
     Create a bot interface and request work
    21 21 
     """
    22 22
    23 
    -import logging
    24 23 
     from pathlib import Path, PurePath
    25 24 
     import sys
    26 25 
     from urllib.parse import urlparse
    ... ... @@ -120,8 +119,7 @@ def cli(context, parent, update_period, remote, client_key, client_cert, server_
    120 119 
             context.cas_client_cert = context.client_cert
    121 120 
             context.cas_server_cert = context.server_cert
    122 121
    123 
    -    context.logger = logging.getLogger(__name__)
    124 
    -    context.logger.debug("Starting for remote {}".format(context.remote))
    122 
    +    click.echo("Starting for remote=[{}]".format(context.remote))
    125 123
    126 124 
         interface = bot_interface.BotInterface(context.channel)
    127 125

  • buildgrid/_app/commands/cmd_cas.py
    ... ... @@ -20,7 +20,6 @@ Execute command
    20 20 
     Request work to be executed and monitor status of jobs.
    21 21 
     """
    22 22
    23 
    -import logging
    24 23 
     import os
    25 24 
     import sys
    26 25 
     from urllib.parse import urlparse
    ... ... @@ -63,8 +62,7 @@ def cli(context, remote, instance_name, client_key, client_cert, server_cert):
    63 62
    64 63 
             context.channel = grpc.secure_channel(context.remote, credentials)
    65 64
    66 
    -    context.logger = logging.getLogger(__name__)
    67 
    -    context.logger.debug("Starting for remote {}".format(context.remote))
    65 
    +    click.echo("Starting for remote=[{}]".format(context.remote))
    68 66
    69 67
    70 68 
     @cli.command('upload-dummy', short_help="Upload a dummy action. Should be used with `execute dummy-request`")
    ... ... @@ -75,7 +73,7 @@ def upload_dummy(context):
    75 73 
             action_digest = uploader.put_message(action)
    76 74
    77 75 
         if action_digest.ByteSize():
    78 
    -        click.echo('Success: Pushed digest "{}/{}"'
    76 
    +        click.echo('Success: Pushed digest=["{}/{}]"'
    79 77 
                        .format(action_digest.hash, action_digest.size_bytes))
    80 78 
         else:
    81 79 
             click.echo("Error: Failed pushing empty message.", err=True)
    ... ... @@ -92,7 +90,7 @@ def upload_file(context, file_path, verify):
    92 90 
             for path in file_path:
    93 91 
                 if not os.path.isabs(path):
    94 92 
                     path = os.path.abspath(path)
    95 
    -            context.logger.debug("Queueing {}".format(path))
    93 
    +            click.echo("Queueing path=[{}]".format(path))
    96 94
    97 95 
                 file_digest = uploader.upload_file(path, queue=True)
    98 96
    ... ... @@ -102,12 +100,12 @@ def upload_file(context, file_path, verify):
    102 100 
         for file_digest in sent_digests:
    103 101 
             file_path = os.path.relpath(files_map[file_digest.hash])
    104 102 
             if verify and file_digest.size_bytes != os.stat(file_path).st_size:
    105 
    -            click.echo('Error: Failed to verify "{}"'.format(file_path), err=True)
    103 
    +            click.echo("Error: Failed to verify '{}'".format(file_path), err=True)
    106 104 
             elif file_digest.ByteSize():
    107 
    -            click.echo('Success: Pushed "{}" with digest "{}/{}"'
    105 
    +            click.echo("Success: Pushed path=[{}] with digest=[{}/{}]"
    108 106 
                            .format(file_path, file_digest.hash, file_digest.size_bytes))
    109 107 
             else:
    110 
    -            click.echo('Error: Failed pushing "{}"'.format(file_path), err=True)
    108 
    +            click.echo("Error: Failed pushing path=[{}]".format(file_path), err=True)
    111 109
    112 110
    113 111 
     @cli.command('upload-dir', short_help="Upload a directory to the CAS server.")
    ... ... @@ -121,7 +119,7 @@ def upload_directory(context, directory_path, verify):
    121 119 
             for node, blob, path in merkle_tree_maker(directory_path):
    122 120 
                 if not os.path.isabs(path):
    123 121 
                     path = os.path.abspath(path)
    124 
    -            context.logger.debug("Queueing {}".format(path))
    122 
    +            click.echo("Queueing path=[{}]".format(path))
    125 123
    126 124 
                 node_digest = uploader.put_blob(blob, digest=node.digest, queue=True)
    127 125
    ... ... @@ -134,12 +132,12 @@ def upload_directory(context, directory_path, verify):
    134 132 
                 node_path = os.path.relpath(node_path)
    135 133 
             if verify and (os.path.isfile(node_path) and
    136 134 
                            node_digest.size_bytes != os.stat(node_path).st_size):
    137 
    -            click.echo('Error: Failed to verify "{}"'.format(node_path), err=True)
    135 
    +            click.echo("Error: Failed to verify path=[{}]".format(node_path), err=True)
    138 136 
             elif node_digest.ByteSize():
    139 
    -            click.echo('Success: Pushed "{}" with digest "{}/{}"'
    137 
    +            click.echo("Success: Pushed path=[{}] with digest=[{}/{}]"
    140 138 
                            .format(node_path, node_digest.hash, node_digest.size_bytes))
    141 139 
             else:
    142 
    -            click.echo('Error: Failed pushing "{}"'.format(node_path), err=True)
    140 
    +            click.echo("Error: Failed pushing path=[{}]".format(node_path), err=True)
    143 141
    144 142
    145 143 
     def _create_digest(digest_string):
    ... ... @@ -160,8 +158,8 @@ def _create_digest(digest_string):
    160 158 
     @pass_context
    161 159 
     def download_file(context, digest_string, file_path, verify):
    162 160 
         if os.path.exists(file_path):
    163 
    -        click.echo('Error: Invalid value for "file-path": ' +
    164 
    -                   'Path "{}" already exists.'.format(file_path), err=True)
    161 
    +        click.echo("Error: Invalid value for " +
    162 
    +                   "path=[{}] already exists.".format(file_path), err=True)
    165 163 
             return
    166 164
    167 165 
         digest = _create_digest(digest_string)
    ... ... @@ -171,11 +169,11 @@ def download_file(context, digest_string, file_path, verify):
    171 169 
         if verify:
    172 170 
             file_digest = create_digest(read_file(file_path))
    173 171 
             if file_digest != digest:
    174 
    -            click.echo('Error: Failed to verify "{}"'.format(file_path), err=True)
    172 
    +            click.echo("Error: Failed to verify path=[{}]".format(file_path), err=True)
    175 173 
                 return
    176 174
    177 175 
         if os.path.isfile(file_path):
    178 
    -        click.echo('Success: Pulled "{}" from digest "{}/{}"'
    176 
    +        click.echo("Success: Pulled path=[{}] from digest=[{}/{}]"
    179 177 
                        .format(file_path, digest.hash, digest.size_bytes))
    180 178 
         else:
    181 179 
             click.echo('Error: Failed pulling "{}"'.format(file_path), err=True)
    ... ... @@ -190,8 +188,8 @@ def download_file(context, digest_string, file_path, verify):
    190 188 
     def download_directory(context, digest_string, directory_path, verify):
    191 189 
         if os.path.exists(directory_path):
    192 190 
             if not os.path.isdir(directory_path) or os.listdir(directory_path):
    193 
    -            click.echo('Error: Invalid value for "directory-path": ' +
    194 
    -                       'Path "{}" already exists.'.format(directory_path), err=True)
    191 
    +            click.echo("Error: Invalid value, " +
    192 
    +                       "path=[{}] already exists.".format(directory_path), err=True)
    195 193 
                 return
    196 194
    197 195 
         digest = _create_digest(digest_string)
    ... ... @@ -204,11 +202,11 @@ def download_directory(context, digest_string, directory_path, verify):
    204 202 
                 if node.DESCRIPTOR is remote_execution_pb2.DirectoryNode.DESCRIPTOR:
    205 203 
                     last_directory_node = node
    206 204 
             if last_directory_node.digest != digest:
    207 
    -            click.echo('Error: Failed to verify "{}"'.format(directory_path), err=True)
    205 
    +            click.echo("Error: Failed to verify path=[{}]".format(directory_path), err=True)
    208 206 
                 return
    209 207
    210 208 
         if os.path.isdir(directory_path):
    211 
    -        click.echo('Success: Pulled "{}" from digest "{}/{}"'
    209 
    +        click.echo("Success: Pulled path=[{}] from digest=[{}/{}]"
    212 210 
                        .format(directory_path, digest.hash, digest.size_bytes))
    213 211 
         else:
    214 
    -        click.echo('Error: Failed pulling "{}"'.format(directory_path), err=True)
    212 
    +        click.echo("Error: Failed pulling path=[{}]".format(directory_path), err=True)

  • buildgrid/_app/commands/cmd_execute.py
    ... ... @@ -20,7 +20,6 @@ Execute command
    20 20 
     Request work to be executed and monitor status of jobs.
    21 21 
     """
    22 22
    23 
    -import logging
    24 23 
     import os
    25 24 
     import stat
    26 25 
     import sys
    ... ... @@ -64,8 +63,7 @@ def cli(context, remote, instance_name, client_key, client_cert, server_cert):
    64 63
    65 64 
             context.channel = grpc.secure_channel(context.remote, credentials)
    66 65
    67 
    -    context.logger = logging.getLogger(__name__)
    68 
    -    context.logger.debug("Starting for remote {}".format(context.remote))
    66 
    +    click.echo("Starting for remote=[{}]".format(context.remote))
    69 67
    70 68
    71 69 
     @cli.command('request-dummy', short_help="Send a dummy action.")
    ... ... @@ -76,7 +74,7 @@ def cli(context, remote, instance_name, client_key, client_cert, server_cert):
    76 74 
     @pass_context
    77 75 
     def request_dummy(context, number, wait_for_completion):
    78 76
    79 
    -    context.logger.info("Sending execution request...")
    77 
    +    click.echo("Sending execution request...")
    80 78 
         action = remote_execution_pb2.Action(do_not_cache=True)
    81 79 
         action_digest = create_digest(action.SerializeToString())
    82 80
    ... ... @@ -96,7 +94,7 @@ def request_dummy(context, number, wait_for_completion):
    96 94 
                 result = None
    97 95 
                 for stream in response:
    98 96 
                     result = stream
    99 
    -                context.logger.info(result)
    97 
    +                click.echo(result)
    100 98
    101 99 
                 if not result.done:
    102 100 
                     click.echo("Result did not return True." +
    ... ... @@ -104,7 +102,7 @@ def request_dummy(context, number, wait_for_completion):
    104 102 
                     sys.exit(-1)
    105 103
    106 104 
             else:
    107 
    -            context.logger.info(next(response))
    105 
    +            click.echo(next(response))
    108 106
    109 107
    110 108 
     @cli.command('command', short_help="Send a command to be executed.")
    ... ... @@ -132,12 +130,12 @@ def run_command(context, input_root, commands, output_file, output_directory):
    132 130
    133 131 
             command_digest = uploader.put_message(command, queue=True)
    134 132
    135 
    -        context.logger.info('Sent command: {}'.format(command_digest))
    133 
    +        click.echo("Sent command=[{}]".format(command_digest))
    136 134
    137 135 
             # TODO: Check for missing blobs
    138 136 
             input_root_digest = uploader.upload_directory(input_root)
    139 137
    140 
    -        context.logger.info('Sent input: {}'.format(input_root_digest))
    138 
    +        click.echo("Sent input=[{}]".format(input_root_digest))
    141 139
    142 140 
             action = remote_execution_pb2.Action(command_digest=command_digest,
    143 141 
                                                  input_root_digest=input_root_digest,
    ... ... @@ -145,7 +143,7 @@ def run_command(context, input_root, commands, output_file, output_directory):
    145 143
    146 144 
             action_digest = uploader.put_message(action, queue=True)
    147 145
    148 
    -        context.logger.info("Sent action: {}".format(action_digest))
    146 
    +        click.echo("Sent action="">".format(action_digest))
    149 147
    150 148 
         request = remote_execution_pb2.ExecuteRequest(instance_name=context.instance_name,
    151 149 
                                                       action_digest=action_digest,
    ... ... @@ -154,7 +152,7 @@ def run_command(context, input_root, commands, output_file, output_directory):
    154 152
    155 153 
         stream = None
    156 154 
         for stream in response:
    157 
    -        context.logger.info(stream)
    155 
    +        click.echo(stream)
    158 156
    159 157 
         execute_response = remote_execution_pb2.ExecuteResponse()
    160 158 
         stream.response.Unpack(execute_response)

  • buildgrid/_app/commands/cmd_operation.py
    ... ... @@ -21,7 +21,6 @@ Check the status of operations
    21 21 
     """
    22 22
    23 23 
     from collections import OrderedDict
    24 
    -import logging
    25 24 
     from operator import attrgetter
    26 25 
     from urllib.parse import urlparse
    27 26 
     import sys
    ... ... @@ -67,8 +66,7 @@ def cli(context, remote, instance_name, client_key, client_cert, server_cert):
    67 66
    68 67 
             context.channel = grpc.secure_channel(context.remote, credentials)
    69 68
    70 
    -    context.logger = logging.getLogger(__name__)
    71 
    -    context.logger.debug("Starting for remote {}".format(context.remote))
    69 
    +    click.echo("Starting for remote=[{}]".format(context.remote))
    72 70
    73 71
    74 72 
     def _print_operation_status(operation, print_details=False):
    ... ... @@ -155,6 +153,20 @@ def status(context, operation_name, json):
    155 153 
             click.echo(json_format.MessageToJson(operation))
    156 154
    157 155
    156 
    +@cli.command('cancel', short_help="Cancel an operation.")
    157 
    +@click.argument('operation-name', nargs=1, type=click.STRING, required=True)
    158 
    +@pass_context
    159 
    +def cancel(context, operation_name):
    160 
    +    clic.echo("Cancelling an operation...")
    161 
    +    stub = operations_pb2_grpc.OperationsStub(context.channel)
    162 
    +
    163 
    +    request = operations_pb2.CancelOperationRequest(name=operation_name)
    164 
    +
    165 
    +    stub.CancelOperation(request)
    166 
    +
    167 
    +    click.echo("Operation cancelled: [{}]".format(request))
    168 
    +
    169 
    +
    158 170 
     @cli.command('list', short_help="List operations.")
    159 171 
     @click.option('--json', is_flag=True, show_default=True,
    160 172 
                   help="Print operations list in JSON format.")

  • buildgrid/_app/commands/cmd_server.py
    ... ... @@ -21,7 +21,6 @@ Create a BuildGrid server.
    21 21 
     """
    22 22
    23 23 
     import asyncio
    24 
    -import logging
    25 24 
     import sys
    26 25
    27 26 
     import click
    ... ... @@ -35,7 +34,7 @@ from ..settings import parser
    35 34 
     @click.group(name='server', short_help="Start a local server instance.")
    36 35 
     @pass_context
    37 36 
     def cli(context):
    38 
    -    context.logger = logging.getLogger(__name__)
    37 
    +    pass
    39 38
    40 39
    41 40 
     @cli.command('start', short_help="Setup a new server instance.")
    ... ... @@ -61,7 +60,7 @@ def start(context, config):
    61 60 
             pass
    62 61
    63 62 
         finally:
    64 
    -        context.logger.info("Stopping server")
    63 
    +        click.echo("Stopping server")
    65 64 
             server.stop()
    66 65 
             loop.close()
    67 66

  • buildgrid/_exceptions.py
    ... ... @@ -52,6 +52,12 @@ class BotError(BgdError):
    52 52 
             super().__init__(message, detail=detail, domain=ErrorDomain.BOT, reason=reason)
    53 53
    54 54
    55 
    +class CancelledError(BgdError):
    56 
    +    """The job was cancelled and any callers should be notified"""
    57 
    +    def __init__(self, message, detail=None, reason=None):
    58 
    +        super().__init__(message, detail=detail, domain=ErrorDomain.SERVER, reason=reason)
    59 
    +
    60 
    +
    55 61 
     class InvalidArgumentError(BgdError):
    56 62 
         """A bad argument was passed, such as a name which doesn't exist."""
    57 63 
         def __init__(self, message, detail=None, reason=None):

  • buildgrid/server/execution/instance.py
    ... ... @@ -72,8 +72,10 @@ class ExecutionInstance:
    72 72 
                 raise InvalidArgumentError("Operation name does not exist: [{}]".format(name))
    73 73
    74 74 
         def stream_operation_updates(self, message_queue, operation_name):
    75 
    -        operation = message_queue.get()
    76 
    -        while not operation.done:
    77 
    -            yield operation
    78 
    -            operation = message_queue.get()
    79 
    -        yield operation
    75 
    +        job = message_queue.get()
    76 
    +        while not job.operation.done:
    77 
    +            yield job.operation
    78 
    +            job = message_queue.get()
    79 
    +            job.check_operation_status()
    80 
    +
    81 
    +        yield job.operation

  • buildgrid/server/execution/service.py
    ... ... @@ -26,7 +26,7 @@ from functools import partial
    26 26
    27 27 
     import grpc
    28 28
    29 
    -from buildgrid._exceptions import FailedPreconditionError, InvalidArgumentError
    29 
    +from buildgrid._exceptions import FailedPreconditionError, InvalidArgumentError, CancelledError
    30 30 
     from buildgrid._protos.build.bazel.remote.execution.v2 import remote_execution_pb2_grpc
    31 31 
     from buildgrid._protos.google.longrunning import operations_pb2
    32 32
    ... ... @@ -79,6 +79,12 @@ class ExecutionService(remote_execution_pb2_grpc.ExecutionServicer):
    79 79 
                 context.set_code(grpc.StatusCode.FAILED_PRECONDITION)
    80 80 
                 yield operations_pb2.Operation()
    81 81
    82 
    +        except CancelledError as e:
    83 
    +            self.__logger.error(e)
    84 
    +            context.set_details(str(e))
    85 
    +            context.set_code(grpc.StatusCode.CANCELLED)
    86 
    +            yield operations_pb2.Operation()
    87 
    +
    82 88 
         def WaitExecution(self, request, context):
    83 89 
             self.__logger.debug("WaitExecution request from [%s]", context.peer())
    84 90
    ... ... @@ -111,6 +117,12 @@ class ExecutionService(remote_execution_pb2_grpc.ExecutionServicer):
    111 117 
                 context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
    112 118 
                 yield operations_pb2.Operation()
    113 119
    120 
    +        except CancelledError as e:
    121 
    +            self.__logger.error(e)
    122 
    +            context.set_details(str(e))
    123 
    +            context.set_code(grpc.StatusCode.CANCELLED)
    124 
    +            yield operations_pb2.Operation()
    125 
    +
    114 126 
         def _get_instance(self, name):
    115 127 
             try:
    116 128 
                 return self._instances[name]

  • buildgrid/server/job.py
    ... ... @@ -19,9 +19,11 @@ import uuid
    19 19 
     from google.protobuf import timestamp_pb2
    20 20
    21 21 
     from buildgrid._enums import LeaseState, OperationStage
    22 
    +from buildgrid._exceptions import CancelledError
    22 23 
     from buildgrid._protos.build.bazel.remote.execution.v2 import remote_execution_pb2
    23 24 
     from buildgrid._protos.google.devtools.remoteworkers.v1test2 import bots_pb2
    24 25 
     from buildgrid._protos.google.longrunning import operations_pb2
    26 
    +from buildgrid._protos.google.rpc import code_pb2
    25 27
    26 28
    27 29 
     class Job:
    ... ... @@ -36,10 +38,14 @@ class Job:
    36 38
    37 39 
             self.__execute_response = None
    38 40 
             self.__operation_metadata = remote_execution_pb2.ExecuteOperationMetadata()
    41 
    +
    39 42 
             self.__queued_timestamp = timestamp_pb2.Timestamp()
    40 43 
             self.__worker_start_timestamp = timestamp_pb2.Timestamp()
    41 44 
             self.__worker_completed_timestamp = timestamp_pb2.Timestamp()
    42 45
    46 
    +        self.__operation_cancelled = False
    47 
    +        self.__lease_cancelled = False
    48 
    +
    43 49 
             self.__operation_metadata.action_digest.CopyFrom(action_digest)
    44 50 
             self.__operation_metadata.stage = OperationStage.UNKNOWN.value
    45 51
    ... ... @@ -103,11 +109,13 @@ class Job:
    103 109 
         def register_client(self, queue):
    104 110 
             """Subscribes to the job's :class:`Operation` stage change events.
    105 111
    112 
    +        Queues this :object:`Job` instance.
    113 
    +
    106 114 
             Args:
    107 115 
                 queue (queue.Queue): the event queue to register.
    108 116 
             """
    109 117 
             self._operation_update_queues.append(queue)
    110 
    -        queue.put(self._operation)
    118 
    +        queue.put(self)
    111 119
    112 120 
         def unregister_client(self, queue):
    113 121 
             """Unsubscribes to the job's :class:`Operation` stage change events.
    ... ... @@ -130,7 +138,9 @@ class Job:
    130 138 
             Only one :class:`Lease` can be emitted for a given job. This method
    131 139 
             should only be used once, any furhter calls are ignored.
    132 140 
             """
    133 
    -        if self._lease is not None:
    141 
    +        if self.__operation_cancelled:
    142 
    +            return None
    143 
    +        elif self._lease is not None:
    134 144 
                 return None
    135 145
    136 146 
             self._lease = bots_pb2.Lease()
    ... ... @@ -171,7 +181,7 @@ class Job:
    171 181 
                 action_result = remote_execution_pb2.ActionResult()
    172 182
    173 183 
                 # TODO: Make a distinction between build and bot failures!
    174 
    -            if status.code != 0:
    184 
    +            if status.code != code_pb2.OK:
    175 185 
                     self._do_not_cache = True
    176 186
    177 187 
                 if result is not None:
    ... ... @@ -188,6 +198,15 @@ class Job:
    188 198 
                 self.__execute_response.cached_result = False
    189 199 
                 self.__execute_response.status.CopyFrom(status)
    190 200
    201 
    +    def cancel_lease(self):
    202 
    +        """Triggers a job's :class:Lease cancellation.
    203 
    +
    204 
    +        This will not cancel the job's :class:Operation.
    205 
    +        """
    206 
    +        self.__lease_cancelled = True
    207 
    +        if self._lease is not None:
    208 
    +            self.update_lease_state(LeaseState.CANCELLED)
    209 
    +
    191 210 
         def update_operation_stage(self, stage):
    192 211 
             """Operates a stage transition for the job's :class:Operation.
    193 212
    ... ... @@ -212,4 +231,28 @@ class Job:
    212 231 
             self._operation.metadata.Pack(self.__operation_metadata)
    213 232
    214 233 
             for queue in self._operation_update_queues:
    215 
    -            queue.put(self._operation)
    234 
    +            queue.put(self)
    235 
    +
    236 
    +    def check_operation_status(self):
    237 
    +        """Reports errors on unexpected job's :class:Operation state.
    238 
    +
    239 
    +        Raises:
    240 
    +            CancelledError: if the job's :class:Operation was cancelled.
    241 
    +        """
    242 
    +        if self.__operation_cancelled:
    243 
    +            raise CancelledError(self.__execute_response.status.message)
    244 
    +
    245 
    +    def cancel_operation(self):
    246 
    +        """Triggers a job's :class:Operation cancellation.
    247 
    +
    248 
    +        This will also cancel any job's :class:Lease that may have been issued.
    249 
    +        """
    250 
    +        self.__operation_cancelled = True
    251 
    +        if self._lease is not None:
    252 
    +            self.cancel_lease()
    253 
    +
    254 
    +        self.__execute_response = remote_execution_pb2.ExecuteResponse()
    255 
    +        self.__execute_response.status.code = code_pb2.CANCELLED
    256 
    +        self.__execute_response.status.message = "Operation cancelled by client."
    257 
    +
    258 
    +        self.update_operation_stage(OperationStage.COMPLETED)

  • buildgrid/server/operations/instance.py
    ... ... @@ -65,6 +65,13 @@ class OperationsInstance:
    65 65 
             except KeyError:
    66 66 
                 raise InvalidArgumentError("Operation name does not exist: [{}]".format(name))
    67 67
    68 
    +    def cancel_operation(self, name):
    69 
    +        try:
    70 
    +            self._scheduler.cancel_job_operation(name)
    71 
    +
    72 
    +        except KeyError:
    73 
    +            raise InvalidArgumentError("Operation name does not exist: [{}]".format(name))
    74 
    +
    68 75 
         def register_message_client(self, name, queue):
    69 76 
             try:
    70 77 
                 self._scheduler.register_client(name, queue)
    ... ... @@ -80,12 +87,10 @@ class OperationsInstance:
    80 87 
                 raise InvalidArgumentError("Operation name does not exist: [{}]".format(name))
    81 88
    82 89 
         def stream_operation_updates(self, message_queue, operation_name):
    83 
    -        operation = message_queue.get()
    84 
    -        while not operation.done:
    85 
    -            yield operation
    86 
    -            operation = message_queue.get()
    87 
    -        yield operation
    90 
    +        job = message_queue.get()
    91 
    +        while not job.operation.done:
    92 
    +            yield job.operation
    93 
    +            job = message_queue.get()
    94 
    +            job.check_operation_status()
    88 95
    89 
    -    def cancel_operation(self, name):
    90 
    -        # TODO: Cancel leases
    91 
    -        raise NotImplementedError("Cancelled operations not supported")
    96 
    +        yield job.operation

  • buildgrid/server/operations/service.py
    ... ... @@ -120,11 +120,6 @@ class OperationsService(operations_pb2_grpc.OperationsServicer):
    120 120 
                 operation_name = self._parse_operation_name(name)
    121 121 
                 instance.cancel_operation(operation_name)
    122 122
    123 
    -        except NotImplementedError as e:
    124 
    -            self.__logger.error(e)
    125 
    -            context.set_details(str(e))
    126 
    -            context.set_code(grpc.StatusCode.UNIMPLEMENTED)
    127 
    -
    128 123 
             except InvalidArgumentError as e:
    129 124 
                 self.__logger.error(e)
    130 125 
                 context.set_details(str(e))

  • buildgrid/server/scheduler.py
    ... ... @@ -97,7 +97,10 @@ class Scheduler:
    97 97 
             # For now, one lease at a time:
    98 98 
             lease = job.create_lease()
    99 99
    100 
    -        return [lease]
    100 
    +        if lease:
    101 
    +            return [lease]
    102 
    +
    103 
    +        return None
    101 104
    102 105 
         def update_job_lease_state(self, job_name, lease_state, lease_status=None, lease_result=None):
    103 106 
             """Requests a state transition for a job's current :class:Lease.
    ... ... @@ -136,3 +139,13 @@ class Scheduler:
    136 139 
         def get_job_operation(self, job_name):
    137 140 
             """Returns the operation associated to job."""
    138 141 
             return self.jobs[job_name].operation
    142 
    +
    143 
    +    def cancel_job_operation(self, job_name):
    144 
    +        """"Cancels the underlying operation of a given job.
    145 
    +
    146 
    +        This will also cancel any job's lease that may have been issued.
    147 
    +
    148 
    +        Args:
    149 
    +            job_name (str): name of the job holding the operation to cancel.
    150 
    +        """
    151 
    +        self.jobs[job_name].cancel_operation()

  • tests/integration/operations_service.py
    ... ... @@ -24,6 +24,7 @@ import grpc
    24 24 
     from grpc._server import _Context
    25 25 
     import pytest
    26 26
    27 
    +from buildgrid._enums import OperationStage
    27 28 
     from buildgrid._exceptions import InvalidArgumentError
    28 29 
     from buildgrid._protos.build.bazel.remote.execution.v2 import remote_execution_pb2
    29 30 
     from buildgrid._protos.google.longrunning import operations_pb2
    ... ... @@ -236,12 +237,24 @@ def test_delete_operation_fail(instance, context):
    236 237 
         context.set_code.assert_called_once_with(grpc.StatusCode.INVALID_ARGUMENT)
    237 238
    238 239
    239 
    -def test_cancel_operation(instance, context):
    240 
    +def test_cancel_operation(instance, controller, execute_request, context):
    241 
    +    response_execute = controller.execution_instance.execute(execute_request.action_digest,
    242 
    +                                                             execute_request.skip_cache_lookup)
    243 
    +
    240 244 
         request = operations_pb2.CancelOperationRequest()
    241 
    -    request.name = "{}/{}".format(instance_name, "runner")
    245 
    +    request.name = "{}/{}".format(instance_name, response_execute.name)
    246 
    +
    242 247 
         instance.CancelOperation(request, context)
    243 248
    244 
    -    context.set_code.assert_called_once_with(grpc.StatusCode.UNIMPLEMENTED)
    249 
    +    request = operations_pb2.ListOperationsRequest(name=instance_name)
    250 
    +    response = instance.ListOperations(request, context)
    251 
    +
    252 
    +    assert len(response.operations) is 1
    253 
    +
    254 
    +    for operation in response.operations:
    255 
    +        operation_metadata = remote_execution_pb2.ExecuteOperationMetadata()
    256 
    +        operation.metadata.Unpack(operation_metadata)
    257 
    +        assert operation_metadata.stage == OperationStage.COMPLETED.value
    245 258
    246 259
    247 260 
     def test_cancel_operation_blank(blank_instance, context):
    ... ... @@ -249,7 +262,7 @@ def test_cancel_operation_blank(blank_instance, context):
    249 262 
         request.name = "runner"
    250 263 
         blank_instance.CancelOperation(request, context)
    251 264
    252 
    -    context.set_code.assert_called_once_with(grpc.StatusCode.UNIMPLEMENTED)
    265 
    +    context.set_code.assert_called_once_with(grpc.StatusCode.INVALID_ARGUMENT)
    253 266
    254 267
    255 268 
     def test_cancel_operation_instance_fail(instance, context):

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.

    [Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]