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.

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

fix beam-runners-flink and zeppelin-scio scala version to 2.10 #3258

Closed
wants to merge 0 commits into from

Conversation

avnerl
Copy link

@avnerl avnerl commented Dec 10, 2018

What is this PR for?

build is failing when trying to use scala-2.11 profile

  • beam-runners-flink 2.0.0 was never built with anything other than scala 2.10, therefore parametrizing its scala version breaks the build.
  • scio is hard coded with scala 2.10, therefore parametrizing its scala version breaks the build.

What type of PR is it?

[Hot Fix]

Todos

  • - merge or handle zeppelin-scio and beam-runner-flink scala versions

What is the Jira issue?

  • N/A

How should this be tested?

  • Pass CI
  • manual build:
mvn clean package -Pbuild-distr -DskipTests -Pspark-2.4 -Phadoop-2.7 -Pscala-2.11

Screenshots (if appropriate)

Questions:

  • no licenses files need update
  • no breaking changes for older versions
  • Does this needs documentation? not sure

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

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

hm, yes, but then building -Pscala-2.11 will provide scala-2.10 output? that's kinda weird.

@avnerl
Copy link
Author

avnerl commented Dec 11, 2018

you are right. it didn't make sense. the reason i did this is because i wanted the build to pass. i remove the beam interpreter afterwards as i dont need it.
last 2 commits revert back to parameterized scala version in beam, and introduced a new beam build profile that bump beam version to 2.3.0 when when building with -Pscala-2.11.

@avnerl
Copy link
Author

avnerl commented Dec 11, 2018

do you think it's a good idea to tamper with beam version in dev/change_scala_version.sh too?
build will not pass if you do not use the -Pscala-2.11 profile explicitly

@felixcheung
Copy link
Member

what about we don't build beam and scio if scala-2.11?

@felixcheung
Copy link
Member

also we could upgrade beam interpreter to "require" beam 2.3 also (so it does build with scala-2.11)

@avnerl
Copy link
Author

avnerl commented Dec 12, 2018

also we could upgrade beam interpreter to "require" beam 2.3 also (so it does build with scala-2.11)

that's what i did in: ffca03c

do you have a better approach?

@felixcheung
Copy link
Member

that's what i did in: ffca03c

ah, that would make different versions of beam with scala-2.10 or scala-2.11.

is there a scio version that works with scala-2.11? what if we upgrade the supported version of beam and scio in all profile?

@avnerl
Copy link
Author

avnerl commented Dec 17, 2018

that's what i did in: ffca03c

ah, that would make different versions of beam with scala-2.10 or scala-2.11.

is there a scio version that works with scala-2.11? what if we upgrade the supported version of beam and scio in all profile?

we don't have much choice here. beam was never crossed built with scala. starting version 2.3.0 it is built with 2.11 only (prior versions with 2.10).
so changing scala version implies changing beam version as well.
scio 0.2.4 is cross built with both 2.10 and 2.11, so it is already being taken care of.

@avnerl
Copy link
Author

avnerl commented Dec 17, 2018

@felixcheung
Copy link
Member

in that case, can we make it build beam only when it has support for it.
ie. build scala 2.10 - beam 2.0.0
build scala 2.11 - beam not included (but not failing)

@avnerl
Copy link
Author

avnerl commented Dec 29, 2018

closing this for lack of time. i'll use this for my own testing:
mvn clean package -Pbuild-distr -DskipTests -Pspark-2.4 -Phadoop-2.7 -Pscala-2.11 -p !beam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants