-
Notifications
You must be signed in to change notification settings - Fork 274
feat: support configuring the manifold URL #968
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
base: main
Are you sure you want to change the base?
Conversation
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. (Because this pull request was imported automatically, there will not be any future comments.) |
d99291a
to
236e347
Compare
This is neat, taking a look |
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.
One thing missing here is that it won't work when downloading logs via --trace-id .
Probably the way forward on that is to actually use our ManifoldClient there instead of relying on shelling out. Having said that, this PR should still work for rage and html, so if you just want to add a TODO there and do it in a different PR that is ok with me
added a couple of nits, feel free to ignore those for now
It actually does work for this, but only because I simply implemented the How would you like to proceed with fixing this after landing this PR though? Should buck2 just fetch them from the fbcdn (and equivalent) URLs in ManifoldClient? Should it implement the actual Manifold GET API (in which case I can't help you because I don't know what it is)? |
2adf680
to
f2d09c4
Compare
Looks like a bogus CI failure (or at least one I have nothing to do with). Surprised it builds internally at Facebook given that I suspect I broke a usage of the API I replaced ( |
I have a highly trivial Manifold implementation internally at Mercury (https://github.com/mercurytechnologies/locally-euclidean) for which I would like to set up for buck2 to receive rage and all that at. This allows for fully implementing the build logs feature mentioned in facebook#441. There's still a buck2_core::facebook_only() or two that need inspected/removed and some more tidying.
f2d09c4
to
8f07fe6
Compare
I have a highly trivial Manifold implementation internally at Mercury for which I would like to set up for buck2 to receive rage and all that at. This allows for fully implementing the build logs feature mentioned in #441.
There's still a buck2_core::facebook_only() or two that need removed and some more tidying.