New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transfer Throw/Noop values across the native boundary #4025

Closed
stuhood opened this Issue Nov 3, 2016 · 2 comments

Comments

Projects
None yet
1 participant
@stuhood
Member

stuhood commented Nov 3, 2016

Currently, the native engine does not attempt to preserve Throw values that cross the boundary between python and rust. Part of the reason for this is that we (re)use a single buffer for the transfer of strings/utf8 data, and the completion of Runnables is batch-based.

One fix might be to make the content of the core::Throw on the rust side a core::Value representing an Exception class. The downside of this is that the rust code would need an extern to call back to python to construct exception objects in case of internal failures; but the upside is that we could preserve exceptions thrown in user code.

@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood Nov 4, 2016

Member

Some tests are disabled due to this, and labeled with 4025

Member

stuhood commented Nov 4, 2016

Some tests are disabled due to this, and labeled with 4025

@stuhood stuhood self-assigned this Nov 7, 2016

@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood Nov 7, 2016

Member

Will take this one.

Member

stuhood commented Nov 7, 2016

Will take this one.

@stuhood stuhood closed this in #4076 Nov 19, 2016

stuhood added a commit that referenced this issue Nov 19, 2016

[engine] Initial Trace implementation for rust engine (#4076)
Problem
----------
Re-implementing trace was deferred during the initial work on the Rust scheduler. See #4007.

Solution
----------
 This patch partially reintroduces it. The rust implementation will now produce traces that include the subjects and some of the information about failures. Some types of failures will not have good messages yet. This is because we haven't landed a patch to fix #4025. Also, the trace function will produce traces for each of the roots.

Result
-------

Example output:

    $ echo "WUT" >> BUILD.tools 
    $ ./pants --enable-v2-engine list //:
    Exception caught: (<class 'pants.build_graph.address_lookup_error.AddressLookupError'>)
      File "/.../pants/src/python/pants/bin/pants_exe.py", line 50, in <module>
        main()
      File "/.../pants/src/python/pants/bin/pants_exe.py", line 44, in main
        PantsRunner(exiter).run()
      File "/.../pants/src/python/pants/bin/pants_runner.py", line 57, in run
        options_bootstrapper=options_bootstrapper)
      File "/.../pants/src/python/pants/bin/pants_runner.py", line 46, in _run
        return LocalPantsRunner(exiter, args, env, options_bootstrapper=options_bootstrapper).run()
      File "/.../pants/src/python/pants/bin/local_pants_runner.py", line 37, in run
        self._run()
      File "/.../pants/src/python/pants/bin/local_pants_runner.py", line 77, in _run
        self._exiter).setup()
      File "/.../pants/src/python/pants/bin/goal_runner.py", line 184, in setup
        goals, context = self._setup_context()
      File "/.../pants/src/python/pants/bin/goal_runner.py", line 157, in _setup_context
        self._daemon_graph_helper
      File "/.../pants/src/python/pants/bin/goal_runner.py", line 101, in _init_graph
        graph, address_mapper = graph_helper.create_build_graph(target_roots, self._root_dir)
      File "/.../pants/src/python/pants/bin/engine_initializer.py", line 88, in create_build_graph
        for _ in graph.inject_specs_closure(target_roots.as_specs()):
      File "/.../pants/src/python/pants/engine/legacy/graph.py", line 205, in inject_specs_closure
        for address in self._inject(specs):
      File "/.../pants/src/python/pants/engine/legacy/graph.py", line 225, in _inject
        self._index(target_entries)
      File "/.../pants/src/python/pants/engine/legacy/graph.py", line 83, in _index
        'Build graph construction failed for {}:\n{}'.format(node, trace))

    Exception message: Build graph construction failed for (SiblingAddresses(directory=u''), Select(Collection.of(HydratedTarget))):
    Computing =Collection.of(HydratedTarget) for SiblingAddresses(directory=u'')
      Computing =Collection.of(HydratedTarget) for SiblingAddresses(directory=u'')
        Computing =HydratedTarget for SiblingAddresses(directory=u'')
          Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
            Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
              Computing =AddressFamily for SiblingAddresses(directory=u'')
                Computing =AddressFamily for Dir(path=u'')
                  Computing =AddressFamily for Dir(path=u'')
                    Throw("EntryId(17) failed!")

    Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
      Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
        Computing =AddressFamily for SiblingAddresses(directory=u'')
          Computing =AddressFamily for Dir(path=u'')
            Computing =AddressFamily for Dir(path=u'')
              Throw("EntryId(17) failed!")


Testing
--------

Local manual testing with ./pants list and a corrupted BUILD file. Running tests that had been skipped due to this feature being missing. CI away on #4076

stuhood added a commit to twitter/pants that referenced this issue Nov 19, 2016

@stuhood stuhood reopened this Nov 19, 2016

stuhood added a commit to twitter/pants that referenced this issue Nov 19, 2016

stuhood added a commit that referenced this issue Nov 22, 2016

Restore propagation of thrown exceptions between rust and python (#4083)
### Problem

In the first implementation of the rust engine, I punted on passing thrown exception values back and forth between rust and python, to avoid dealing with string manipulation. A fair number of tests were disabled in order to account for that.

### Solution

Rust `Throw` states now hold a `Value` representing a python exception, and a new extern was created to allow rust to instantiate an Exception for errors occurring in scheduling.

### Result

Throw values always contain python objects, #4025 is fixed, and all tests that were disabled for #4025 are now reenabled.

@stuhood stuhood closed this Nov 29, 2016

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