Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove obsolete setup-velox-torcharrow.sh #8961

Closed
wants to merge 1 commit into from
Closed

Remove obsolete setup-velox-torcharrow.sh #8961

wants to merge 1 commit into from

Conversation

pedroerp
Copy link
Contributor

@pedroerp pedroerp commented Mar 5, 2024

According to discussion on:
#8950 (comment)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 5, 2024
@netlify Netlify
Copy link

netlify bot commented Mar 5, 2024 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 78e85e7
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/65e744e3c9677a00088dc187

@pedroerp pedroerp requested review from kgpai and mbasmanova March 5, 2024 16:15
@pedroerp pedroerp mentioned this pull request Mar 5, 2024
Closed
mbasmanova
mbasmanova approved these changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the cleanup.

@facebook-github-bot
Copy link
Contributor

@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@pedroerp merged this pull request in a3e6717.

@majetideepak
Copy link
Collaborator

I see setup-velox-torcharrow.sh being used here

velox/scripts/velox-torcharrow-container.dockfile

Line 19 in f696b56

COPY scripts/setup-velox-torcharrow.sh /

@majetideepak
Copy link
Collaborator

I found this since CI failed in my PR https://github.com/facebookincubator/velox/actions/runs/8167751354/job/22328669824?pr=8943

@majetideepak majetideepak mentioned this pull request Mar 6, 2024
Closed
@pedroerp
Copy link
Contributor Author

pedroerp commented Mar 6, 2024

I found this since CI failed in my PR https://github.com/facebookincubator/velox/actions/runs/8167751354/job/22328669824?pr=8943

Ughh, looks like the torcharrow docker image generated is used by pyvelox. I reached out to you guys on Slack to figure out what to do with it.

@kgpai
Copy link
Contributor

kgpai commented Mar 6, 2024

We need to remove some of these references to torcharrow , Its not required anymore. - this includes the torcharrow dockerimage which uses manylinux2014 which is going eol in june. We will also need to remove the pyvelox wheel jobs that use these scripts/images etc.

Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Jun 7, 2024
@pedroerp @Joe-Abraham
Remove obsolete setup-velox-torcharrow.sh (facebookincubator#8961)
d1d45cb 
Summary:
According to discussion on:
facebookincubator#8950 (comment)

Pull Request resolved: facebookincubator#8961

Reviewed By: mbasmanova

Differential Revision: D54552525

Pulled By: pedroerp

fbshipit-source-id: 8e07a7e15d6794394e2cfa7cab67e5d0a82a2fb8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbasmanova mbasmanova mbasmanova approved these changes

@kgpai kgpai Awaiting requested review from kgpai

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pedroerp @facebook-github-bot @majetideepak @kgpai @mbasmanova