fix: Fixing compile pip requirements when running on an RBE #2479
Conversation
| # replace or move files across filesystems. | ||
| os.replace = shutil.copy | ||
| os.replace = shutil.move |
There was a problem hiding this comment.
FYI, I am looking at the PR that made the change previously (#1053) and I can't see why one wouldn't want the shutil.move. I also can't see looking at the source of shutil.move if it would be problematic, it will also try to fallback to shutil.copy2, so I think the shutil.move is a robust solution here.
python/private/pypi/pip_compile.bzl
Outdated
| @@ -21,6 +21,8 @@ make it possible to have multiple tools inside the `pypi` directory | |||
|
|
|||
| load("//python:defs.bzl", _py_binary = "py_binary", _py_test = "py_test") | |||
|
|
|||
| _DEFAULT_TAGS = ["no-sandbox", "no-remote-exec", "requires-network"] | |||
There was a problem hiding this comment.
I am not sure if we want to drop the requires-network tag if someone specifies their own tags, e.g. manual. That said, I think I see the intention - allow one to drop the no-sandbox and no-remote-exec tags.
Please consider how to implement this differently.
There was a problem hiding this comment.
I'm a bit curious, too, why one would want to drop these tags. Doesn't pip_compile more-or-less assume it's being run using bazel run (i.e its local, does network stuff, and intends to update local files) ?
@shs96c are you, perhaps using bazel build to create requirements using e.g. RBE? (I like that idea)
There was a problem hiding this comment.
Hah, totally forgot what the title says: ... when running on RBE
Looking at pip_compile, I'm not even sure how you got that working. I don't see anything that runs the tool as an action.
There was a problem hiding this comment.
We have some other wrappers around this that we use, but the core thing we do is to call pip_compile
There was a problem hiding this comment.
I'm not sure of a nicer way of handling the tags, but I've added a remoteable parameter, and will append tag values based on that. If you'd like this handled in some other way, just LMK and I'll make the change.
There was a problem hiding this comment.
@aignas, if you've any advice on how to handle tags in the way you'd like, please let me know!
We have observed that making this change makes builds more likely to run successfully on an RBE.