Skip to content
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

Backtrack through calls from @rules to synchronous engine methods #15979

Merged
merged 5 commits into from
Jun 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/python/pants/base/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@

from __future__ import annotations

from typing import TYPE_CHECKING

if TYPE_CHECKING:
from pants.engine.internals.native_engine import PyFailure


class TargetDefinitionException(Exception):
"""Indicates an invalid target definition.
Expand All @@ -28,3 +33,15 @@ class BackendConfigurationError(BuildConfigurationError):

class MappingError(Exception):
"""Indicates an error mapping addressable objects."""


class NativeEngineFailure(Exception):
"""A wrapper around a `Failure` instance.

TODO: This type is defined in Python because pyo3 doesn't support declaring Exceptions with
additional fields.
stuhood marked this conversation as resolved.
Show resolved Hide resolved
"""

def __init__(self, msg: str, failure: PyFailure) -> None:
super().__init__(msg)
self.failure = failure
6 changes: 6 additions & 0 deletions src/python/pants/engine/internals/native_engine.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ from pants.engine.process import InteractiveProcessResult
# see https://github.com/psf/black/issues/1548
# flake8: noqa: E302

# ------------------------------------------------------------------------------
# (core)
# ------------------------------------------------------------------------------

class PyFailure: ...

# ------------------------------------------------------------------------------
# Address (parsing)
# ------------------------------------------------------------------------------
Expand Down
5 changes: 4 additions & 1 deletion src/rust/engine/src/externs/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ use fs::{
use hashing::{Digest, Fingerprint, EMPTY_DIGEST};
use store::Snapshot;

use crate::Failure;

pub(crate) fn register(m: &PyModule) -> PyResult<()> {
m.add_class::<PyDigest>()?;
m.add_class::<PyFileDigest>()?;
Expand All @@ -40,7 +42,8 @@ pub(crate) fn register(m: &PyModule) -> PyResult<()> {
// Exception type out of `@rule` bodies and back into `Failure::MissingDigest`, to allow for retry
// via #11331.
pub fn todo_possible_store_missing_digest(e: store::StoreError) -> PyErr {
PyException::new_err(e.to_string())
let failure: Failure = e.into();
failure.into()
}

#[pyclass(name = "Digest")]
Expand Down
3 changes: 2 additions & 1 deletion src/rust/engine/src/externs/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ use crate::{

#[pymodule]
fn native_engine(py: Python, m: &PyModule) -> PyO3Result<()> {
externs::register(py, m)?;
externs::address::register(py, m)?;
externs::fs::register(m)?;
externs::nailgun::register(py, m)?;
Expand All @@ -65,9 +66,9 @@ fn native_engine(py: Python, m: &PyModule) -> PyO3Result<()> {

m.add_class::<PyExecutionRequest>()?;
m.add_class::<PyExecutionStrategyOptions>()?;
m.add_class::<PyLocalStoreOptions>()?;
m.add_class::<PyNailgunServer>()?;
m.add_class::<PyRemotingOptions>()?;
m.add_class::<PyLocalStoreOptions>()?;
m.add_class::<PyResult>()?;
m.add_class::<PyScheduler>()?;
m.add_class::<PySession>()?;
Expand Down
14 changes: 14 additions & 0 deletions src/rust/engine/src/externs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::fmt;

use lazy_static::lazy_static;
use pyo3::exceptions::PyException;
use pyo3::import_exception;
use pyo3::prelude::*;
use pyo3::types::{PyBytes, PyDict, PyTuple, PyType};
use pyo3::{FromPyObject, ToPyObject};
Expand All @@ -31,6 +32,19 @@ mod stdio;
pub mod testutil;
pub mod workunits;

pub fn register(_py: Python, m: &PyModule) -> PyResult<()> {
m.add_class::<PyFailure>()?;
Ok(())
}

#[derive(Clone)]
#[pyclass]
pub struct PyFailure(pub Failure);

// TODO: We import this exception type because `pyo3` doesn't support declaring exceptions with
stuhood marked this conversation as resolved.
Show resolved Hide resolved
// additional fields.
import_exception!(pants.base.exceptions, NativeEngineFailure);

pub fn equals(h1: &PyAny, h2: &PyAny) -> bool {
// NB: Although it does not precisely align with Python's definition of equality, we ban matches
// between non-equal types to avoid legacy behavior like `assert True == 1`, which is very
Expand Down
16 changes: 16 additions & 0 deletions src/rust/engine/src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,16 @@ impl Failure {
}

pub fn from_py_err_with_gil(py: Python, py_err: PyErr) -> Failure {
// If this is a wrapped Failure, return it immediately.
if let Ok(n_e_failure) = py_err.value(py).downcast::<externs::NativeEngineFailure>() {
let failure = n_e_failure
.getattr("failure")
.unwrap()
.extract::<externs::PyFailure>()
.unwrap();
return failure.0;
}

let maybe_ptraceback = py_err
.traceback(py)
.map(|traceback| traceback.to_object(py));
Expand Down Expand Up @@ -500,6 +510,12 @@ impl From<StoreError> for Failure {
}
}

impl From<Failure> for PyErr {
fn from(err: Failure) -> Self {
externs::NativeEngineFailure::new_err((err.to_string(), externs::PyFailure(err)))
}
}

impl From<String> for Failure {
fn from(err: String) -> Self {
throw(err)
Expand Down