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

Rules consuming Pex's should respect the Pex's target platform. #8384

Open
hrfuller opened this issue Oct 2, 2019 · 2 comments
Open

Rules consuming Pex's should respect the Pex's target platform. #8384

hrfuller opened this issue Oct 2, 2019 · 2 comments
Labels
backend: Python Python backend-related issues

Comments

@hrfuller
Copy link
Member

hrfuller commented Oct 2, 2019

As a follow up to #8090 rules that consume platform dependent output (Pex for instance) should respect the target_platform that it was built for. Currently we have the ability to restrict the execution platform of a request, but the target_platform is not respected. For instance the run_python_test rule creates a pytest Pex. That Pex is built on the local platform but if speculation is enabled it could be shipped and run on the remote platform, which if different would result in a stochastic error.

To prevent this we need to add a target platform field to the Pex class that consuming rules can respect. Then execution they request can happen on the platform that the Pex is compatible with. Once we have #7490 and Platforms are yieldable in Get's we can request Pex's for multiple platforms and make the run_python_test rule multi platform compatible.

@stuhood
Copy link
Member

stuhood commented Oct 3, 2019

It would be good to expand the design doc (and link it here) to account for this case: we probably want to fix this more generally via the graph, rather than necessarily fixing it only for Pex.

@hrfuller
Copy link
Member Author

hrfuller commented Oct 3, 2019

Agreed. I think the "fixing via the graph" is the same as #8353 which already has speculation on inputs as a follow up. Dependency wise I think we need:

That will allow this to be fixed correctly.

@gshuflin gshuflin self-assigned this Feb 12, 2020
@cognifloyd cognifloyd added the backend: Python Python backend-related issues label Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues
Projects
None yet
Development

No branches or pull requests

4 participants