Skip to content
This repository has been archived by the owner on Mar 16, 2022. It is now read-only.

scala support #116

Merged
merged 10 commits into from Oct 9, 2019
Merged

scala support #116

merged 10 commits into from Oct 9, 2019

Conversation

janory
Copy link
Contributor

@janory janory commented Sep 30, 2019

Steps:

  • copy the java-support ✅
  • make it work as it is ✅
  • extract the common parts (as discusses in the meeting separate PR)
  • change annotation based support, to more idiomatic (as discusses in the meeting separate PR)
  • add CRDT support (as discusses in the meeting separate PR)

@viktorklang
Copy link
Contributor

Hmmm, weird CI failure, could be linked to Coursier: sbt/sbt#4296 Will have to dig deeper.

@janory
Copy link
Contributor Author

janory commented Oct 1, 2019

Hmmm, weird CI failure, could be linked to Coursier: sbt/sbt#4296 Will have to dig deeper.

Yeah, it seems unrelated to the code change.

Edit: Ahh... I've checked the linked sbt issue, so it's because of the scala version.

@@ -627,6 +627,70 @@ lazy val `java-support` = (project in file("java-support"))
)
)

lazy val `scala-support` = (project in file("scala-support"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Try addding this project to aggregate:

.aggregate(`proxy-core`,
             `proxy-cassandra`,
             `proxy-postgres`,
             `java-support`,
             `scala-support`, <--- HERE
             `java-shopping-cart`,
             `akka-client`,
             operator,
             `tck`,
             docs)
  .settings(common)

@janory
Copy link
Contributor Author

janory commented Oct 1, 2019

@viktorklang

I am currently having an issue with the assembly of the scala-shopping-cart:

[error] stack trace is suppressed; run last scala-shopping-cart / assembly for the full output
[error] (scala-shopping-cart / assembly) deduplicate: different file contents found in the following:
[error] com/google/api/AnnotationsProto.class
[error] com/google/api/AnnotationsProto.class
[error] deduplicate: different file contents found in the following:
[error] com/google/api/CustomHttpPattern.class
[error] com/google/api/CustomHttpPattern.class
[error] deduplicate: different file contents found in the following:
[error] com/google/api/Http.class
[error] com/google/api/Http.class
[error] deduplicate: different file contents found in the following:
[error] com/google/api/HttpProto.class
[error] com/google/api/HttpProto.class
[error] deduplicate: different file contents found in the following:
[error] com/google/api/HttpRule.class
[error] com/google/api/HttpRule.class

My gut feeling says, that the scala-support module already generates these files and because scala-shopping-cart is depending on that, it is generated two times.
But if I remove the baseDir / "frontend" from the scala-shopping-cart the assembly also fails, because the classes are not generated at all.

Edit: It's maybe a local issue, because Travis is not complaining.

PB.generate in Compile := (PB.generate in Compile).dependsOn(PB.generate in (`scala-support`, Compile)).value,
PB.protoSources in Compile ++= {
val baseDir = (baseDirectory in ThisBuild).value / "protocols"
Seq(baseDir / "frontend", baseDir / "example")
Copy link
Contributor

Choose a reason for hiding this comment

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

@janory Have you tried

Suggested change
Seq(baseDir / "frontend", baseDir / "example")
Seq(baseDir / "example")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@viktorklang
I've checked out the branch & run the TCK on a different computer and it worked well, so I will cleanup the project and try it again. It is definitely a local issue.

The TCK is almost complete, the only issue is this:

[info] - must verify that the HTTP API of ShoppingCart protocol works *** FAILED ***
"...ems":[{"productId":"[B14623482","name":"Basic","quantity":1},{"productId":"A14362347","name":"Deluxe","quantity":7]}]}" did not equal "...ems":[{"productId":"[A14362347","name":"Deluxe","quantity":7},{"productId":"B14623482","name":"Basic","quantity":1]}]}" 

It is the same content, but the order is different.
I will check the implementation and fix the order, although maybe we can relax the TCK's check to ignore the order and focus only the content, because my guess is, that the order here is not important.

Copy link
Contributor

@marcellanz marcellanz Oct 2, 2019

Choose a reason for hiding this comment

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

@janory I had the excactly same "issue" in the Go Support first :-), had the items in a list that is not ordered.

Copy link
Contributor Author

@janory janory Oct 2, 2019

Choose a reason for hiding this comment

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

@marcellanz :)
It's a one liner fix, but I am just not sure, if we really need to enforce the order here.

Copy link
Contributor

@marcellanz marcellanz Oct 2, 2019

Choose a reason for hiding this comment

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

Had the same thought, yes.
There are other implicit requirements by the shopping cart tck example. Adding negative amounts of line-items, or better not allowing it, as an example. As the shopping cart is used to verify the spec through the TCK, we perhaps might document these requirements within the shoppingcart.proto file.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason for checking the response as a String was to avoid having parsing influencing things needlessly. I'm very much open for a better way of verifying the responses :)

@viktorklang
Copy link
Contributor

@janory Ok, so the next steps is to create a Scala-idiomatic API, and then once that's done, to see what commonalities still exist between the java and scala support, and pull those out into a jvm-support library?

@janory
Copy link
Contributor Author

janory commented Oct 9, 2019

@janory Ok, so the next steps is to create a Scala-idiomatic API, and then once that's done, to see what commonalities still exist between the java and scala support, and pull those out into a jvm-support library?

@viktorklang exactly! But, I think those should happen in separate PRs.

"org.slf4j" % "slf4j-simple" % "1.7.26",
"com.fasterxml.jackson.core" % "jackson-databind" % "2.9.9.3"
),
javacOptions in Compile ++= Seq("-encoding", "UTF-8"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this to the other javacOptiuons above?

@viktorklang viktorklang merged commit 95dc369 into cloudstateio:master Oct 9, 2019
@viktorklang
Copy link
Contributor

@janory Cool, let the next step begin! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants