-
Notifications
You must be signed in to change notification settings - Fork 340
[RFC] Fix reviewstack.dev deploy by removing broken Docker container dependency #1208
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
|
@ajaymenon has updated the pull request. You must reimport the pull request before landing. |
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D92435020. (Because this pull request was imported automatically, there will not be any future comments.) |
bherila
left a comment
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.
Could I request you restore the comments and remove trailing whitespace, I will check tomorrow to see how we can validate the diff. I think it is OK however, just these nits :)
|
Btw thank you for your contribution to reviewstack! |
|
@ajaymenon has updated the pull request. You must reimport the pull request before landing. |
|
Also still getting used to Github as opposed to Phabricator - I've made the requested changes and was able to re-request review, but it still says the PR has changes requested. |
|
Thanks, I will get to it today anyway! |
|
@bherila - it seems that Merging is blocked because "Merge is not an allowed merge method in this repository". Not quite sure how else I'm supposed to commit this PR. Would you be willing to take it over? The reason the local build+deploy method doesn't solve my problem is because my colleagues are the ones trying to use reviewstack to review my PRs internally. |
|
Because I made a change to this diff internally, I need an additional review from another employee. If any Meta employee sees this and can merge the diff, please do! I will ping some more people today if not. |
|
By the way @ajaymenon
but I'm asking around! (fyi, Reviewstack is a "extracurricular" project, It's not my primary job.) |
|
I would love to make a new PR right now but the github outages are making that impossible on my side. Also no pressure I get it's a side thing - appreciate your followups here! @ashwinb maybe you could help approve this PR from the other side? |
|
Nvm was able to make #1216 |
Summary
The repo used to have a CI workflow that built an Ubuntu 22.04 Docker image (generated by
ci/gen_workflows.pyassapling-cli-ubuntu-22.04-image.yml) and pushed it to GitHub Container Registry asghcr.io/facebook/sapling/build_ubuntu_22_04:latest. At some point, that workflow and its Dockerfile were removed from the repo, but two workflows that depend on that image were never updated:reviewstack.dev-deploy.ymlsapling-addons.ymlSo every time either of those workflows runs, it tries to pull an image that no longer exists, and fails immediately at container initialization.
Example failure on
main: https://github.com/facebook/sapling/actions/runs/21693937235/job/62559938248Test Plan
Not 100% sure how to test this tbh. I tried to add a trigger to get GHA to pick up the change and see if the deploy action would succeed but it looks like I don't have permissions to do that.
ubuntu-22.04 should already have node and yarn so the build deployment should work without any custom dependencies.