fix: Fixing compile pip requirements when running on an RBE #2479
Conversation
| # Replace the os.replace function with shutil.move to work around os.replace not being able to | ||
| # 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
|
|
||
| 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!
3 participants
We have observed that making this change makes builds more likely to run successfully on an RBE.