Jürg Billeter pushed to branch valentindavid/wrong_type_in_status_code at BuildStream / buildstream
Commits:
- 
a3e2cdd2
by Tom Pollard at 2019-01-28T12:14:40Z
 - 
805baf7d
by Tom Pollard at 2019-01-28T12:14:40Z
 - 
38356932
by Tom Pollard at 2019-01-28T13:44:37Z
 - 
3590ca8c
by Abderrahim Kitouni at 2019-01-28T14:59:19Z
 - 
39b952dc
by Abderrahim Kitouni at 2019-01-28T15:19:04Z
 - 
80b36d0c
by Javier Jardón at 2019-01-28T17:54:52Z
 - 
a1ab48da
by Valentin David at 2019-01-28T21:30:26Z
 - 
2fcb4491
by Jürg Billeter at 2019-01-28T22:36:22Z
 - 
1c05a092
by Valentin David at 2019-01-29T05:58:17Z
 
6 changed files:
- buildstream/_cas/casserver.py
 - buildstream/_frontend/cli.py
 - buildstream/_scheduler/scheduler.py
 - buildstream/_stream.py
 - requirements/requirements.in
 - tests/integration/build-tree.py
 
Changes:
| ... | ... | @@ -324,7 +324,7 @@ class _ContentAddressableStorageServicer(remote_execution_pb2_grpc.ContentAddres | 
| 324 | 324 | 
             blob_response.digest.size_bytes = digest.size_bytes
 | 
| 325 | 325 | 
 | 
| 326 | 326 | 
             if len(blob_request.data) != digest.size_bytes:
 | 
| 327 | 
-                blob_response.status.code = grpc.StatusCode.FAILED_PRECONDITION
 | 
|
| 327 | 
+                blob_response.status.code = code_pb2.FAILED_PRECONDITION
 | 
|
| 328 | 328 | 
                 continue
 | 
| 329 | 329 | 
 | 
| 330 | 330 | 
             try:
 | 
| ... | ... | @@ -335,10 +335,10 @@ class _ContentAddressableStorageServicer(remote_execution_pb2_grpc.ContentAddres | 
| 335 | 335 | 
                     out.flush()
 | 
| 336 | 336 | 
                     server_digest = self.cas.add_object(path=out.name)
 | 
| 337 | 337 | 
                     if server_digest.hash != digest.hash:
 | 
| 338 | 
-                        blob_response.status.code = grpc.StatusCode.FAILED_PRECONDITION
 | 
|
| 338 | 
+                        blob_response.status.code = code_pb2.FAILED_PRECONDITION
 | 
|
| 339 | 339 | 
 | 
| 340 | 340 | 
             except ArtifactTooLargeException:
 | 
| 341 | 
-                blob_response.status.code = grpc.StatusCode.RESOURCE_EXHAUSTED
 | 
|
| 341 | 
+                blob_response.status.code = code_pb2.RESOURCE_EXHAUSTED
 | 
|
| 342 | 342 | 
 | 
| 343 | 343 | 
         return response
 | 
| 344 | 344 | 
 | 
| ... | ... | @@ -526,7 +526,7 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) | 
| 526 | 526 | 
     else:
 | 
| 527 | 527 | 
         scope = Scope.RUN
 | 
| 528 | 528 | 
 | 
| 529 | 
-    use_buildtree = False
 | 
|
| 529 | 
+    use_buildtree = None
 | 
|
| 530 | 530 | 
 | 
| 531 | 531 | 
     with app.initialized():
 | 
| 532 | 532 | 
         if not element:
 | 
| ... | ... | @@ -534,7 +534,8 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) | 
| 534 | 534 | 
             if not element:
 | 
| 535 | 535 | 
                 raise AppError('Missing argument "ELEMENT".')
 | 
| 536 | 536 | 
 | 
| 537 | 
-        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE)
 | 
|
| 537 | 
+        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE,
 | 
|
| 538 | 
+                                                 use_artifact_config=True)
 | 
|
| 538 | 539 | 
         element = dependencies[0]
 | 
| 539 | 540 | 
         prompt = app.shell_prompt(element)
 | 
| 540 | 541 | 
         mounts = [
 | 
| ... | ... | @@ -543,20 +544,31 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) | 
| 543 | 544 | 
         ]
 | 
| 544 | 545 | 
 | 
| 545 | 546 | 
         cached = element._cached_buildtree()
 | 
| 546 | 
-        if cli_buildtree == "always":
 | 
|
| 547 | 
-            if cached:
 | 
|
| 548 | 
-                use_buildtree = True
 | 
|
| 549 | 
-            else:
 | 
|
| 550 | 
-                raise AppError("No buildtree is cached but the use buildtree option was specified")
 | 
|
| 551 | 
-        elif cli_buildtree == "never":
 | 
|
| 552 | 
-            pass
 | 
|
| 553 | 
-        elif cli_buildtree == "try":
 | 
|
| 554 | 
-            use_buildtree = cached
 | 
|
| 547 | 
+        if cli_buildtree in ("always", "try"):
 | 
|
| 548 | 
+            use_buildtree = cli_buildtree
 | 
|
| 549 | 
+            if not cached and use_buildtree == "always":
 | 
|
| 550 | 
+                click.echo("WARNING: buildtree is not cached locally, will attempt to pull from available remotes",
 | 
|
| 551 | 
+                           err=True)
 | 
|
| 555 | 552 | 
         else:
 | 
| 556 | 
-            if app.interactive and cached:
 | 
|
| 557 | 
-                use_buildtree = bool(click.confirm('Do you want to use the cached buildtree?'))
 | 
|
| 553 | 
+            # If the value has defaulted to ask and in non interactive mode, don't consider the buildtree, this
 | 
|
| 554 | 
+            # being the default behaviour of the command
 | 
|
| 555 | 
+            if app.interactive and cli_buildtree == "ask":
 | 
|
| 556 | 
+                if cached and bool(click.confirm('Do you want to use the cached buildtree?')):
 | 
|
| 557 | 
+                    use_buildtree = "always"
 | 
|
| 558 | 
+                elif not cached:
 | 
|
| 559 | 
+                    try:
 | 
|
| 560 | 
+                        choice = click.prompt("Do you want to pull & use a cached buildtree?",
 | 
|
| 561 | 
+                                              type=click.Choice(['try', 'always', 'never']),
 | 
|
| 562 | 
+                                              err=True, show_choices=True)
 | 
|
| 563 | 
+                    except click.Abort:
 | 
|
| 564 | 
+                        click.echo('Aborting', err=True)
 | 
|
| 565 | 
+                        sys.exit(-1)
 | 
|
| 566 | 
+  | 
|
| 567 | 
+                    if choice != "never":
 | 
|
| 568 | 
+                        use_buildtree = choice
 | 
|
| 569 | 
+  | 
|
| 558 | 570 | 
         if use_buildtree and not element._cached_success():
 | 
| 559 | 
-            click.echo("Warning: using a buildtree from a failed build.")
 | 
|
| 571 | 
+            click.echo("WARNING: using a buildtree from a failed build.", err=True)
 | 
|
| 560 | 572 | 
 | 
| 561 | 573 | 
         try:
 | 
| 562 | 574 | 
             exitcode = app.stream.shell(element, scope, prompt,
 | 
| ... | ... | @@ -314,10 +314,10 @@ class Scheduler(): | 
| 314 | 314 | 
     #    job (Job): The job to spawn
 | 
| 315 | 315 | 
     #
 | 
| 316 | 316 | 
     def _spawn_job(self, job):
 | 
| 317 | 
-        job.spawn()
 | 
|
| 318 | 317 | 
         self._active_jobs.append(job)
 | 
| 319 | 318 | 
         if self._job_start_callback:
 | 
| 320 | 319 | 
             self._job_start_callback(job)
 | 
| 320 | 
+        job.spawn()
 | 
|
| 321 | 321 | 
 | 
| 322 | 322 | 
     # Callback for the cache size job
 | 
| 323 | 323 | 
     def _cache_size_job_complete(self, status, cache_size):
 | 
| ... | ... | @@ -101,19 +101,22 @@ class Stream(): | 
| 101 | 101 | 
     #    targets (list of str): Targets to pull
 | 
| 102 | 102 | 
     #    selection (PipelineSelection): The selection mode for the specified targets
 | 
| 103 | 103 | 
     #    except_targets (list of str): Specified targets to except from fetching
 | 
| 104 | 
+    #    use_artifact_config (bool): If artifact remote config should be loaded
 | 
|
| 104 | 105 | 
     #
 | 
| 105 | 106 | 
     # Returns:
 | 
| 106 | 107 | 
     #    (list of Element): The selected elements
 | 
| 107 | 108 | 
     def load_selection(self, targets, *,
 | 
| 108 | 109 | 
                        selection=PipelineSelection.NONE,
 | 
| 109 | 
-                       except_targets=()):
 | 
|
| 110 | 
+                       except_targets=(),
 | 
|
| 111 | 
+                       use_artifact_config=False):
 | 
|
| 110 | 112 | 
 | 
| 111 | 113 | 
         profile_start(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
 | 
| 112 | 114 | 
 | 
| 113 | 115 | 
         elements, _ = self._load(targets, (),
 | 
| 114 | 116 | 
                                  selection=selection,
 | 
| 115 | 117 | 
                                  except_targets=except_targets,
 | 
| 116 | 
-                                 fetch_subprojects=False)
 | 
|
| 118 | 
+                                 fetch_subprojects=False,
 | 
|
| 119 | 
+                                 use_artifact_config=use_artifact_config)
 | 
|
| 117 | 120 | 
 | 
| 118 | 121 | 
         profile_end(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
 | 
| 119 | 122 | 
 | 
| ... | ... | @@ -131,7 +134,7 @@ class Stream(): | 
| 131 | 134 | 
     #    mounts (list of HostMount): Additional directories to mount into the sandbox
 | 
| 132 | 135 | 
     #    isolate (bool): Whether to isolate the environment like we do in builds
 | 
| 133 | 136 | 
     #    command (list): An argv to launch in the sandbox, or None
 | 
| 134 | 
-    #    usebuildtree (bool): Wheather to use a buildtree as the source.
 | 
|
| 137 | 
+    #    usebuildtree (str): Whether to use a buildtree as the source, given cli option
 | 
|
| 135 | 138 | 
     #
 | 
| 136 | 139 | 
     # Returns:
 | 
| 137 | 140 | 
     #    (int): The exit code of the launched shell
 | 
| ... | ... | @@ -141,7 +144,7 @@ class Stream(): | 
| 141 | 144 | 
               mounts=None,
 | 
| 142 | 145 | 
               isolate=False,
 | 
| 143 | 146 | 
               command=None,
 | 
| 144 | 
-              usebuildtree=False):
 | 
|
| 147 | 
+              usebuildtree=None):
 | 
|
| 145 | 148 | 
 | 
| 146 | 149 | 
         # Assert we have everything we need built, unless the directory is specified
 | 
| 147 | 150 | 
         # in which case we just blindly trust the directory, using the element
 | 
| ... | ... | @@ -156,8 +159,31 @@ class Stream(): | 
| 156 | 159 | 
                 raise StreamError("Elements need to be built or downloaded before staging a shell environment",
 | 
| 157 | 160 | 
                                   detail="\n".join(missing_deps))
 | 
| 158 | 161 | 
 | 
| 162 | 
+        buildtree = False
 | 
|
| 163 | 
+        # Check if we require a pull queue attempt, with given artifact state and context
 | 
|
| 164 | 
+        if usebuildtree:
 | 
|
| 165 | 
+            if not element._cached_buildtree():
 | 
|
| 166 | 
+                require_buildtree = self._buildtree_pull_required([element])
 | 
|
| 167 | 
+                # Attempt a pull queue for the given element if remote and context allow it
 | 
|
| 168 | 
+                if require_buildtree:
 | 
|
| 169 | 
+                    self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtree")
 | 
|
| 170 | 
+                    self._add_queue(PullQueue(self._scheduler))
 | 
|
| 171 | 
+                    self._enqueue_plan(require_buildtree)
 | 
|
| 172 | 
+                    self._run()
 | 
|
| 173 | 
+                    # Now check if the buildtree was successfully fetched
 | 
|
| 174 | 
+                    if element._cached_buildtree():
 | 
|
| 175 | 
+                        buildtree = True
 | 
|
| 176 | 
+                if not buildtree:
 | 
|
| 177 | 
+                    if usebuildtree == "always":
 | 
|
| 178 | 
+                        raise StreamError("Buildtree is not cached locally or in available remotes")
 | 
|
| 179 | 
+                    else:
 | 
|
| 180 | 
+                        self._message(MessageType.INFO, """Buildtree is not cached locally or in available remotes,
 | 
|
| 181 | 
+                                                        shell will be loaded without it""")
 | 
|
| 182 | 
+            else:
 | 
|
| 183 | 
+                buildtree = True
 | 
|
| 184 | 
+  | 
|
| 159 | 185 | 
         return element._shell(scope, directory, mounts=mounts, isolate=isolate, prompt=prompt, command=command,
 | 
| 160 | 
-                              usebuildtree=usebuildtree)
 | 
|
| 186 | 
+                              usebuildtree=buildtree)
 | 
|
| 161 | 187 | 
 | 
| 162 | 188 | 
     # build()
 | 
| 163 | 189 | 
     #
 | 
| 1 | 
-Click
 | 
|
| 1 | 
+Click >= 7.0
 | 
|
| 2 | 2 | 
 grpcio >= 1.10
 | 
| 3 | 3 | 
 Jinja2 >= 2.10
 | 
| 4 | 4 | 
 pluginbase
 | 
| 5 | 
-protobuf >= 3.5
 | 
|
| 5 | 
+protobuf >= 3.6
 | 
|
| 6 | 6 | 
 psutil
 | 
| 7 | 7 | 
 # According to ruamel.yaml's PyPI page, we are suppose to use
 | 
| 8 | 8 | 
 # "<=0.15" in production until 0.15 becomes API stable.
 | 
| ... | ... | @@ -101,7 +101,7 @@ def test_buildtree_from_failure(cli_integration, tmpdir, datafiles): | 
| 101 | 101 | 
         'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
 | 
| 102 | 102 | 
     ])
 | 
| 103 | 103 | 
     res.assert_success()
 | 
| 104 | 
-    assert "Warning: using a buildtree from a failed build" in res.output
 | 
|
| 104 | 
+    assert "WARNING: using a buildtree from a failed build" in res.stderr
 | 
|
| 105 | 105 | 
     assert 'Hi' in res.output
 | 
| 106 | 106 | 
 | 
| 107 | 107 | 
 | 
| ... | ... | @@ -141,7 +141,7 @@ def test_buildtree_pulled(cli, tmpdir, datafiles): | 
| 141 | 141 | 
         res.assert_success()
 | 
| 142 | 142 | 
 | 
| 143 | 143 | 
 | 
| 144 | 
-# This test checks for correct behaviour if a buildtree is not present.
 | 
|
| 144 | 
+# This test checks for correct behaviour if a buildtree is not present in the local cache.
 | 
|
| 145 | 145 | 
 @pytest.mark.datafiles(DATA_DIR)
 | 
| 146 | 146 | 
 @pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
 | 
| 147 | 147 | 
 def test_buildtree_options(cli, tmpdir, datafiles):
 | 
| ... | ... | @@ -156,6 +156,7 @@ def test_buildtree_options(cli, tmpdir, datafiles): | 
| 156 | 156 | 
         result = cli.run(project=project, args=['build', element_name])
 | 
| 157 | 157 | 
         result.assert_success()
 | 
| 158 | 158 | 
         assert cli.get_element_state(project, element_name) == 'cached'
 | 
| 159 | 
+        assert share.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 | 
|
| 159 | 160 | 
 | 
| 160 | 161 | 
         # Discard the cache
 | 
| 161 | 162 | 
         cli.configure({
 | 
| ... | ... | @@ -168,8 +169,6 @@ def test_buildtree_options(cli, tmpdir, datafiles): | 
| 168 | 169 | 
         result = cli.run(project=project, args=['artifact', 'pull', '--deps', 'all', element_name])
 | 
| 169 | 170 | 
         result.assert_success()
 | 
| 170 | 171 | 
 | 
| 171 | 
-        # The above is the simplest way I know to create a local cache without any buildtrees.
 | 
|
| 172 | 
-  | 
|
| 173 | 172 | 
         # Check it's not using the cached build tree
 | 
| 174 | 173 | 
         res = cli.run(project=project, args=[
 | 
| 175 | 174 | 
             'shell', '--build', element_name, '--use-buildtree', 'never', '--', 'cat', 'test'
 | 
| ... | ... | @@ -177,24 +176,51 @@ def test_buildtree_options(cli, tmpdir, datafiles): | 
| 177 | 176 | 
         res.assert_shell_error()
 | 
| 178 | 177 | 
         assert 'Hi' not in res.output
 | 
| 179 | 178 | 
 | 
| 180 | 
-        # Check it's not correctly handling the lack of buildtree
 | 
|
| 179 | 
+        # Check it's not using the cached build tree, default is to ask, and fall back to not
 | 
|
| 180 | 
+        # for non interactive behavior
 | 
|
| 181 | 181 | 
         res = cli.run(project=project, args=[
 | 
| 182 | 
-            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 | 
|
| 182 | 
+            'shell', '--build', element_name, '--', 'cat', 'test'
 | 
|
| 183 | 183 | 
         ])
 | 
| 184 | 184 | 
         res.assert_shell_error()
 | 
| 185 | 185 | 
         assert 'Hi' not in res.output
 | 
| 186 | 186 | 
 | 
| 187 | 
-        # Check it's not using the cached build tree, default is to ask, and fall back to not
 | 
|
| 188 | 
-        # for non interactive behavior
 | 
|
| 187 | 
+        # Check correctly handling the lack of buildtree, with 'try' not attempting to
 | 
|
| 188 | 
+        # pull the buildtree as the user context is by default set to not pull them
 | 
|
| 189 | 189 | 
         res = cli.run(project=project, args=[
 | 
| 190 | 
-            'shell', '--build', element_name, '--', 'cat', 'test'
 | 
|
| 190 | 
+            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 | 
|
| 191 | 191 | 
         ])
 | 
| 192 | 
-        res.assert_shell_error()
 | 
|
| 193 | 192 | 
         assert 'Hi' not in res.output
 | 
| 193 | 
+        assert 'Attempting to fetch missing artifact buildtrees' not in res.stderr
 | 
|
| 194 | 
+        assert """Buildtree is not cached locally or in available remotes,
 | 
|
| 195 | 
+                shell will be loaded without it"""
 | 
|
| 194 | 196 | 
 | 
| 195 | 
-        # Check it's using the cached build tree
 | 
|
| 197 | 
+        # Check correctly handling the lack of buildtree, with 'try' attempting and succeeding
 | 
|
| 198 | 
+        # to pull the buildtree as the user context allow the pulling of buildtrees and it is
 | 
|
| 199 | 
+        # available in the remote
 | 
|
| 200 | 
+        res = cli.run(project=project, args=[
 | 
|
| 201 | 
+            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 | 
|
| 202 | 
+        ])
 | 
|
| 203 | 
+        assert 'Attempting to fetch missing artifact buildtree' in res.stderr
 | 
|
| 204 | 
+        assert 'Hi' in res.output
 | 
|
| 205 | 
+        shutil.rmtree(os.path.join(os.path.join(cli.directory, 'artifacts2')))
 | 
|
| 206 | 
+        assert cli.get_element_state(project, element_name) != 'cached'
 | 
|
| 207 | 
+  | 
|
| 208 | 
+        # Check it's not loading the shell at all with always set for the buildtree, when the
 | 
|
| 209 | 
+        # user context does not allow for buildtree pulling
 | 
|
| 210 | 
+        result = cli.run(project=project, args=['artifact', 'pull', '--deps', 'all', element_name])
 | 
|
| 211 | 
+        result.assert_success()
 | 
|
| 196 | 212 | 
         res = cli.run(project=project, args=[
 | 
| 197 | 213 | 
             'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
 | 
| 198 | 214 | 
         ])
 | 
| 199 | 215 | 
         res.assert_main_error(ErrorDomain.PROG_NOT_FOUND, None)
 | 
| 216 | 
+        assert 'Buildtree is not cached locally or in available remotes' in res.stderr
 | 
|
| 200 | 217 | 
         assert 'Hi' not in res.output
 | 
| 218 | 
+        assert 'Attempting to fetch missing artifact buildtree' not in res.stderr
 | 
|
| 219 | 
+  | 
|
| 220 | 
+        # Check that when user context is set to pull buildtrees and a remote has the buildtree,
 | 
|
| 221 | 
+        # 'always' will attempt and succeed at pulling the missing buildtree.
 | 
|
| 222 | 
+        res = cli.run(project=project, args=[
 | 
|
| 223 | 
+            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
 | 
|
| 224 | 
+        ])
 | 
|
| 225 | 
+        assert 'Hi' in res.output
 | 
|
| 226 | 
+        assert 'Attempting to fetch missing artifact buildtree' in res.stderr
 | 
