Skip to content

Update Arrow Package with breaking changes#2440

Merged
pcmoritz merged 7 commits intoray-project:masterfrom
antgroup:updateArrow
Jul 25, 2018
Merged

Update Arrow Package with breaking changes#2440
pcmoritz merged 7 commits intoray-project:masterfrom
antgroup:updateArrow

Conversation

@guoyuhong
Copy link
Contributor

What do these changes do?

There are 2 breaking changes in Arrow Plasma: apache/arrow#2242 and apache/arrow#2282.
Two header files are unavailable now: plasma.h and protocol.h, but the plasma_manager needs them. I did following things to make this updating smooth.

  1. Removing plasma.fbs from ray: instead, we will copy plasma.fbs and common.fbs from Arrow, which will avoid duplication.
  2. Giving forward declaration for the functions used in the plasma_manager that declared in protocol.h which we cannot use now.

Related issue number

@guoyuhong
Copy link
Contributor Author

@pcmoritz Hi Philipp, there are 2 breaking changes in Arrow Plasma. I'm doing some [WIP] test but found there are conflicts. Could you please give a quick review to unblock my work?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6735/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6739/
Test FAILed.

@guoyuhong guoyuhong closed this Jul 20, 2018
@guoyuhong guoyuhong deleted the updateArrow branch July 20, 2018 15:11
@guoyuhong guoyuhong restored the updateArrow branch July 20, 2018 15:11
@guoyuhong guoyuhong reopened this Jul 20, 2018
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6760/
Test PASSed.

@guoyuhong
Copy link
Contributor Author

@robertnishihara Could you help to review?

@robertnishihara robertnishihara requested a review from pcmoritz July 23, 2018 23:59
@robertnishihara
Copy link
Collaborator

Thanks @guoyuhong! @pcmoritz or I will be able to take a look later tonight.

@pcmoritz
Copy link
Contributor

I'm reviewing it now, sorry for the delay!

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6786/
Test FAILed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In https://github.com/apache/arrow/pull/2282/files , this file is removed from the installation list, so this file is not available now.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yeah, it's a new file with forward declarations that I forgot to add :)

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6799/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6805/
Test FAILed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6816/
Test PASSed.

Copy link
Contributor

@pcmoritz pcmoritz left a comment

Choose a reason for hiding this comment

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

+1 Thanks for doing this!

@pcmoritz pcmoritz merged commit b35ce5d into ray-project:master Jul 25, 2018
@guoyuhong guoyuhong deleted the updateArrow branch July 26, 2018 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments