#193: Drop support of Spark 2.4 #196

benedeki · 2024-05-15T09:12:02Z

dropping Spark 2.4 support
strict use of sbt projectmatrix plugin, tohave cross-builds
basic settings moved to project/Setup.scala class
some move or project setup code to fall into logical areas

Closes #193

* dropping Spark 2.4 support * strict use of sbt projectmatrix plugin, tohave cross-builds * basic settings moved to `project/Setup.scala` class * some move or project setup code to fall into logical areas

github-actions · 2024-05-15T09:14:14Z

JaCoCo server module code coverage report - scala 2.13.11

Overall Project	76.61%	🍏

There is no coverage information present for the Files changed

salamonpavel · 2024-05-15T10:51:04Z

build.sbt


 ThisBuild / versionScheme := Some("early-semver")

 Global / onChangedBuildSource := ReloadOnSourceChanges

 publish / skip := true

-lazy val printSparkScalaVersion = taskKey[Unit]("Print Spark and Scala versions for atum-service is being built for.")
-lazy val printScalaVersion = taskKey[Unit]("Print Scala versions for atum-service is being built for.")
+initialize := {


Is the assignment necessary?

To be honest I don't know. This code block is a remnant of my tries to keep Spakr 2.4 support. And I found it interesting, so kept it for now, with the intention to ask you what you think to have that information (of the active Java version) there.
Just forgot to comment accordingly.
So what do you think, should we keep the info (with the commented out cod removed)?

I think it's useful to have a log statement with the Java version in use. See an alternative implementation below.

val checkJavaVersion = taskKey[Unit]("Check Java version") checkJavaVersion := { val javaVersionUsed = VersionNumber(sys.props("java.specification.version")) streams.value.log.info(s"Running on Java version $javaVersionUsed") } // Run the task when the project is loaded Global / onLoad := (Global / onLoad).value.andThen(state => "checkJavaVersion" :: state)

(base) AB032LP@LFGQVV4V6D atum-service % sbt compile [info] welcome to sbt 1.9.7 (Amazon.com Inc. Java 11.0.22) [info] loading global plugins from /Users/AB032LP/.sbt/1.0/plugins [info] loading settings for project atum-service-build-build-build from metals.sbt ... [info] loading project definition from /Users/AB032LP/IdeaProjects/atum-service/project/project/project [info] loading settings for project atum-service-build-build from metals.sbt ... [info] loading project definition from /Users/AB032LP/IdeaProjects/atum-service/project/project [success] Generated .bloop/atum-service-build-build.json [success] Total time: 1 s, completed 16 May 2024, 10:05:28 [info] loading settings for project atum-service-build from metals.sbt,plugins.sbt ... [info] loading project definition from /Users/AB032LP/IdeaProjects/atum-service/project [success] Generated .bloop/atum-service-build.json [success] Total time: 1 s, completed 16 May 2024, 10:05:29 [info] loading settings for project atum-service from build.sbt,publish.sbt ... [info] set current project to atum-service (in build file:/Users/AB032LP/IdeaProjects/atum-service/) [info] Running on Java version 11 [success] Total time: 0 s, completed 16 May 2024, 10:05:38 [info] Executing in batch mode. For better performance use sbt's shell [info] Building atum-database with Scala 2.13.11 [info] Building atum-model with Scala 2.12.18 [info] Building atum-server with Scala 2.13.11 [info] Building atum-model with Scala 2.13.11 [info] Building atum-agent with Spark 3.3.2, Scala 2.12.18 [info] Building atum-agent with Spark 3.3.2, Scala 2.13.11 [success] Total time: 4 s, completed 16 May 2024, 10:05:42

salamonpavel · 2024-05-15T10:54:10Z

build.sbt

+  val _ = initialize.value // Ensure previous initializations are run
+
+  val requiredJavaVersion = VersionNumber("17")
+  val current = VersionNumber(sys.props("java.specification.version"))


Consider renaming to something like javaVersion or javaVersionInUse.

If code kept, will rename.

salamonpavel · 2024-05-15T10:55:02Z

build.sbt

+initialize := {
+  val _ = initialize.value // Ensure previous initializations are run
+
+  val requiredJavaVersion = VersionNumber("17")


It is currently not applied, but in general, can you explain the intended requirement for Java 17?

Hope the explanation above make it cleared. This would be removed for sure (or maybe used in the information 🤔 )

salamonpavel · 2024-05-15T10:57:19Z

project/Setup.scala

+  val scala212 = "2.12.18"
+  val scala213 = "2.13.11"
+
+  val serviceScalaVersion: String = scala213


Maybe we could align the naming as we sometimes use server and sometimes service. I would suggest to use server everywhere given it's a name of the module.

I think, the original idea had been) (discussed bakc than with @lsulak ) that:
server = REST server
service = all parts of the running service (REST server + database)
If it's not consistent somewhere though, definitely wrong.

Alright, I comprehend the logic behind it. However, without prior knowledge, it might not be immediately clear. Hence, I would suggest being more descriptive, for instance, using a name like "serverAndDatabaseScalaVersion".

salamonpavel · 2024-05-15T11:03:33Z

project/VersionAxes.scala

+      )
+    }
+
+    def singleRow(scalaVersion: String,


Another candidate for renaming as the method's name doesn't tell me anything about what it does.

This is actually a brilliant idea. I was following the projectmatrix name from examples and how it is used in other examples. But I admit, it took me a while to grasp the concept. A better name would make it much easier to understand 👍

salamonpavel · 2024-05-15T11:04:43Z

project/VersionAxes.scala

+      }
+    }
+
+    def scalasRow(scalaVersions: Seq[String],


As below, not sure what the name suggests.

salamonpavel

The build.sbt file is definitely more readable and tidier. Apart from couple of suggested renamings I would argue that even the code in VersionAxis object could be enhanced by few comments so that one immediately knows what a given method does without looking at the implementation.

build.sbt

project/Dependencies.scala

project/Setup.scala

lsulak · 2024-05-15T16:10:29Z

The CI file, i.e. build instructions, must be also changed I assume, with the current setup.

Command sbt projects renders this

[info] agent_spark3
[info] agent_spark32_12
[info] * atum-service
[info] database
[info] model
[info] model2_12
[info] server

I.e. there is no agent (one reason why CI is currently failing) and no suffix means scala 2.13 is used - otherwise, agent and model, defines 2.12 explicitly. Shouldn't we make it mode consistent? Something like

[info] agent_spark32_13
[info] agent_spark32_12
[info] * atum-service2_13
[info] database2_13
[info] model2_13
[info] model2_12
[info] server2_13

❓

--

I think that 2.13 is default = ThisBuild / scalaVersion := Setup.scala213 // default version
and thus is not explicitly used for the module names. I couldn't make it work yet (the explicit suffix)

project/VersionAxes.scala

Co-authored-by: Ladislav Sulak <[email protected]>

benedeki · 2024-05-15T16:56:01Z

The CI file, i.e. build instructions, must be also changed I assume, with the current setup.

Command sbt projects renders this

[info] agent_spark3 [info] agent_spark32_12 [info] * atum-service [info] database [info] model [info] model2_12 [info] server

I.e. there is no agent (one reason why CI is currently failing) and no suffix means scala 2.13 is used - otherwise, agent and model, defines 2.12 explicitly. Shouldn't we make it mode consistent? Something like

[info] agent_spark32_13 [info] agent_spark32_12 [info] * atum-service2_13 [info] database2_13 [info] model2_13 [info] model2_12 [info] server2_13

❓

--

I think that 2.13 is default = ThisBuild / scalaVersion := Setup.scala213 // default version and thus is not explicitly used for the module names. I couldn't make it work yet (the explicit suffix)

Yes, GitHub actions still need changes. I didn't want to hold this one back on that though. I am actually considering to make it a separate PR.

benedeki · 2024-05-15T17:09:37Z

project/Setup.scala

+ // val clientScalacOptions: Seq[String] = Seq("-Ymacro-annotations")
+
+  val serverMergeStrategy = assembly / assemblyMergeStrategy := {
+    case PathList("META-INF", "services", xs @ _*) => MergeStrategy.filterDistinctLines


This xs @ _* is marked by IntelliJ with warning. (I just blindly moved it from build.sbt. What does it actually means @salamonpavel, please?

xs is an alias, it's not needed here as we do not reference the alias and could be removed and the _* expands the elements of the list

* enhanced README.md to describe possible Spark 2.4 build.sbt * increase version of sbt projectMatrix plugin

Zejnilovic · 2024-05-16T09:51:15Z

project/VersionAxes.scala

+      }
+    }
+
+    def addScalaCrossBuild(scalaVersions: Seq[String],


addScalaCrossBuild and addSingleScalaBuild look the same. Why not use one in the other?

* cleared unused an commented out code from project setup files * moved publish related values into `publish.sbt`

* assembly plugin version upgraded * commonSettings moved to Setup.scala

project/Setup.scala

lsulak

Last thing I found about the license header, otherwise it looks good to me, even the sbt assembly works! I'd prefer if CI is fixed in this step as it's not gonna be complicated, right? But up to you, good job!

Co-authored-by: Ladislav Sulak <[email protected]>

#193: Drop support of Spark 2.4

b79f87a

* dropping Spark 2.4 support * strict use of sbt projectmatrix plugin, tohave cross-builds * basic settings moved to `project/Setup.scala` class * some move or project setup code to fall into logical areas

benedeki added the work in progress Work on this item is not yet finished (mainly intended for PRs) label May 15, 2024

benedeki self-assigned this May 15, 2024

benedeki requested review from lsulak, TebaleloS, Zejnilovic, dk1844 and salamonpavel as code owners May 15, 2024 09:12

salamonpavel reviewed May 15, 2024

View reviewed changes

Merge branch 'master' into feature/193-drop-support-of-spark-2.4

3993207