Maven plugin implementation #1049

andreaTP · 2021-05-11T17:59:38Z

This is the first implementation of a Maven plugin to use with Cloudflow applications.

Note:

Depends on: Add an option to dump the logs to an output file fabric8io/docker-maven-plugin#1472 to be merged and released
The Docker images defined in the Maven examples are stripped to minimum working examples and are not full-fledged

core/cloudflow-cr-generator/src/main/scala/cloudflow/cr/Generator.scala

debasishg · 2021-05-13T04:44:38Z

core/cloudflow-cr-generator/src/main/scala/cloudflow/cr/Generator.scala

+      projectId: String,
+      version: String,
+      blueprintStr: String,
+      streamlets: Map[String, com.typesafe.config.Config],


nitpick: avoiding fully qualified path name : see last comment

debasishg · 2021-05-13T04:45:42Z

core/cloudflow-cr-generator/src/main/scala/cloudflow/cr/Generator.scala

+      version: String,
+      blueprintStr: String,
+      streamlets: Map[String, com.typesafe.config.Config],
+      dockerImages: Map[String, String]): String = {


May be return a Try[String] instead of throwing exceptions ?

The error messages are quite nice from Maven in case we directly throw exceptions!

But what happens if downstream fails to catch those nice exceptions ? :-)

downstream fails to catch those nice exceptions

I'm expecting downstream to NOT catch those exceptions and to be propagated up to the JVM (which craches), this is the contract, isn't it?

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala

debasishg

some minor suggestions

core/cloudflow-cr-generator/src/main/scala/cloudflow/cr/Generator.scala

andreaTP · 2021-05-14T13:00:22Z

core/cloudflow-cr-generator/src/main/scala/cloudflow/cr/Generator.scala

+      projectId: String,
+      version: String,
+      blueprintStr: String,
+      streamlets: Map[String, com.typesafe.config.Config],


andreaTP · 2021-05-14T13:00:55Z

core/cloudflow-cr-generator/src/main/scala/cloudflow/cr/Generator.scala

+      version: String,
+      blueprintStr: String,
+      streamlets: Map[String, com.typesafe.config.Config],
+      dockerImages: Map[String, String]): String = {


The error messages are quite nice from Maven in case we directly throw exceptions!

core/cloudflow-it/kafka-cluster.yaml

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala

debasishg · 2021-05-17T05:41:42Z

.../cloudflow-build-support/src/main/scala/com/lightbend/cloudflow/buildtool/KafkaSupport.scala

+        val c = getClass().getClassLoader()
+        Thread.currentThread().setContextClassLoader(c)
+
+        val k = new KafkaContainer(tcutility.DockerImageName.parse("confluentinc/cp-kafka:5.4.3"))


Maybe externalize the image name ?

Can the parse fail / throw exceptions ?

not with a fixed string, if it succeeds once it succeed always I assume

debasishg · 2021-05-17T05:49:34Z

core/cloudflow-build-support/src/main/scala/com/lightbend/cloudflow/buildtool/Scaffold.scala

+  def getDescriptorsOrFail[T](descriptors: Iterable[(T, Try[RuntimeDescriptor])])(
+      error: String => Unit): Iterable[(T, RuntimeDescriptor)] = {


The function has no semantics of failure as part of its type signature. Should we name the function getDescriptorsOrFail or change it simply to getDescriptors ? It throws but usually we don't add that as a semantics in the name of the function. One option would be to change the signature to Try[Iterable[.. and wrap the exception in a Try ..

Those functions have been extracted all "as-is" from the sbt plugin.
This is not fresh new code being added to the code-base.

also is good to throw the exception to the user in those cases, not much we can do with it.
By handling it gracefully we are going just to mask the original error

debasishg · 2021-05-17T05:51:48Z

core/cloudflow-build-support/src/main/scala/com/lightbend/cloudflow/buildtool/Scaffold.scala

+    localFile
+  }
+
+  def createDirs(prefix: String): (Path, Path) = {


directory creation can fail. Should we do Try[(Path, Path)] ?

This looks really aggressive defensive programming 😛
If directory creation fails I think is correct to send to the user a stack-trace with the original exception (that is what happens here)

andreaTP · 2021-05-18T11:48:39Z

Blocked by the release of docker-maven-plugin 0.36.0:
fabric8io/docker-maven-plugin#1472

debasishg

LGTM

RayRoestenburg · 2021-05-19T08:02:00Z

.gitignore

@@ -59,6 +59,7 @@ binaries
 .cache/
 docs-source/test-ui-site.yml
 output/


Why is output dir in gitignore anyway?

RayRoestenburg · 2021-05-19T08:06:01Z

core/cloudflow-build-support/src/main/scala/com/lightbend/cloudflow/buildtool/AppLayout.scala

+ * Copyright (C) 2021 Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package com.lightbend.cloudflow.buildtool


why not stick with cloudflow as root package?

RayRoestenburg · 2021-05-19T08:09:22Z

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala

+import scala.collection.JavaConverters._
+import scala.util.Try
+
+object CloudflowAggregator {


Suggested change

object CloudflowAggregator {

object CloudflowProjectAggregator {

docs/shared-content-source/docs/modules/develop/pages/maven-support.adoc

RayRoestenburg

Some suggestions for the docs. We should also be a bit more clear about the fact that the user has to write their 'dockerfile in maven xml' at this point, as is shown in the examples, (and we likely need to improve this in the future)

…pport.adoc Co-authored-by: Raymond Roestenburg <raymond.roestenburg@gmail.com>

…nitial-maven

Co-authored-by: Raymond Roestenburg <raymond.roestenburg@gmail.com>

…nitial-maven

andreaTP requested review from debasishg and RayRoestenburg May 11, 2021 17:59

debasishg reviewed May 13, 2021

View reviewed changes

core/cloudflow-cr-generator/src/main/scala/cloudflow/cr/Generator.scala Outdated Show resolved Hide resolved

debasishg reviewed May 13, 2021

View reviewed changes

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala Outdated Show resolved Hide resolved

debasishg reviewed May 13, 2021

View reviewed changes

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala Outdated Show resolved Hide resolved

debasishg reviewed May 13, 2021

View reviewed changes

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala Outdated Show resolved Hide resolved

debasishg reviewed May 13, 2021

View reviewed changes

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala Outdated Show resolved Hide resolved

debasishg reviewed May 13, 2021

View reviewed changes

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala Outdated Show resolved Hide resolved

debasishg reviewed May 13, 2021

View reviewed changes

...loudflow-maven-plugin/src/main/scala/com/lightbend/cloudflow/maven/CloudflowAggregator.scala Outdated Show resolved Hide resolved

debasishg reviewed May 13, 2021

View reviewed changes

andreaTP commented May 14, 2021

View reviewed changes

andreaTP marked this pull request as ready for review May 14, 2021 14:58

andreaTP changed the title ~~Initial maven plugin implementation~~ Maven plugin implementation May 14, 2021

debasishg reviewed May 17, 2021

View reviewed changes

andreaTP added 12 commits May 18, 2021 12:30

extract a CR generator application

ddfbd5f

review

da1ae26

map instead of function

a907ded

First working version of a Maven build-app

0c6753e

refactoring to prepare run-local

8018cae

going on to run-local

24448da

first working local-run

3b4b0f3

minor

e58417c

working run-local taxi-ride

563bef8

taxi-ride working

c2a1405

spark not yet deployable but run-local

c1c344e

call-record-aggregator running

530a62a

andreaTP added 3 commits May 18, 2021 12:31

add a section on using Maven

4a9fe11

cleanup

d6d229f

more prod ready docker images for maven builds

04f71be

andreaTP force-pushed the initial-maven branch from 6ab4c1b to 04f71be Compare May 18, 2021 11:31

debasishg approved these changes May 19, 2021

View reviewed changes