-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
Allow Extra Args to be passed to FaaS Pex Build #20939
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good, I think it's just some minor tweaks.
Thank you!
Ah, can you call this out in the release notes? Please add a sentence or paragraph to the Python section of Here's how we phrased it for the
|
(I've updated the PR title and description too, to be a bit more specific about what and why, feel free to edit some more!) |
add unit tests internal: sort more tuples to potentially improve cache hit rates. (pantsbuild#20767) I looked through most `tuple` uses in the Python backend, and added `sorted(..)` where I think the order wasn't already stable and the result ends up as a rule output. update docs and test fixture
d9199e3
to
0432327
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!
06674ae
to
3a437e2
Compare
Linter passed for me locally but I think I fixed what was causing the fail in CI now. |
There was another merge conflict on the release notes, so I took the liberty of resolving it. |
Thanks for the contribution! |
…ss globally (#21202) A new option `[pex-cli].global_args` has been added to be able to pass arbitrary arguments to the `pex` tool as part of any Pants goal invocation. This should make it a lot easier to modify behavior of `pex` tool without needing to make changes in the Pants codebase. Not having this ability to pass arbitrary arguments to pex makes it really hard to start taking advantage of new features that come with newer versions of pex. For instance, the new `--exclude` flag added recently would require making lots of changes in the codebase to be able to pass those extra arguments. This is because the pex invocations in the Pants codebase are numerous and it's really hard to make sure a particular argument is respected (by keeping the chain of calls correct making sure it does reach the final subprocess spawn). And if it's relevant for multiple goals, this becomes even harder. We would still need to make changes to pass arguments to individual targets, see #20737 or #20939 - this makes sense as those arguments apply only to those targets. However, some options would need to apply for any `pex` invocation (e.g. ignoring all 3rd party dependencies). I've looked into having environment variables support for all flags that PEX has (pex-tool/pex#2242) first (so that no changes are needed in Pants, one would just export a bunch of env vars as needed), but this is not going to happen any time soon, so doing it in the Pants codebase instead is the only path forward, I reckon.
Similar to #20237, this allows passing arbitrary arguments to the first pex invocation when building a FaaS (AWS Lambda or Google Cloud Function) artifact.
This can serve as a stop-gap for #19256, because a user can write then pass
--exclude
themselves when they really need that (or similar) functionality.