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

Rename package and conf to pekko #19

Merged
merged 1 commit into from
Mar 27, 2023

Conversation

mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Mar 24, 2023

So the PR is now ready, some things to note:

  • There is an issue with jackson dependencies that should probably be looked into afterwards (see Rename package and conf to pekko #19 (review))
  • Projection deals with SQL like databases (such as cassandra and JDBC) which involves schemas. This means that a lot of schema's also had to be updated (originally they had akka_ prefix, now have pekko_ prefix)
  • Conf and package changes needed to be done at once because of now using Pekko dependencies rather than akka
  • Imports have also been cleaned up (they don't use FQCN and the render of documentation looks correct, checked locally)

@mdedetrich mdedetrich marked this pull request as draft March 24, 2023 23:04
@mdedetrich mdedetrich force-pushed the rename-package-to-pekko branch 9 times, most recently from 52d0a2c to 209cfc6 Compare March 25, 2023 09:39
@mdedetrich mdedetrich changed the title Rename package to pekko Rename package and conf to pekko Mar 25, 2023
@jrudolph
Copy link
Contributor

This diff should resolve the dependency problems:

diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index 98eb607..41ca262 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -89,6 +89,8 @@ object Dependencies {
   object Examples {
     val hibernate = "org.hibernate" % "hibernate-core" % "5.4.33"
 
+    val pekkoPersistenceQuery = "org.apache.pekko" %% "pekko-persistence-query" % Versions.pekko
+    val pekkoPersistence = "org.apache.pekko" %% "pekko-persistence" % Versions.pekko
     val pekkoPersistenceTyped = "org.apache.pekko" %% "pekko-persistence-typed" % Versions.pekko
     val pekkoClusterShardingTyped = "org.apache.pekko" %% "pekko-cluster-sharding-typed" % Versions.pekko
     val pekkoPersistenceCassandra = "org.apache.pekko" %% "pekko-persistence-cassandra" % "0.0.0-1071-8529fe4f-SNAPSHOT"
@@ -189,12 +191,15 @@ object Dependencies {
   val examples =
     deps ++= Seq(
       Examples.pekkoPersistenceTyped,
+      Examples.pekkoPersistenceQuery,
+      Examples.pekkoPersistence,
       Examples.pekkoClusterShardingTyped,
       Examples.pekkoPersistenceCassandra,
       Examples.pekkoPersistenceJdbc,
       Examples.pekkoSerializationJackson,
       Examples.hibernate,
       Test.h2Driver,
+      Test.pekkoStreamTestkit,
       Test.pekkoTypedTestkit,
       Test.logback,
       Test.cassandraContainer)

@mdedetrich
Copy link
Contributor Author

mdedetrich commented Mar 27, 2023

This diff should resolve the dependency problems:

I am not that familiar with Pekko projection, does it make sense to do this change outside of the context of this problem specifically (or in other words is not having the changes you are pointing out an oversight?)

The reason why I didn't want to do any such changes right now is I wanted to keep the changes to a minimum, at least until the major restructurings to the project are done (which is what this PR is doing). Can definitely do this change once all of the other stuff is handled.

@jrudolph
Copy link
Contributor

jrudolph commented Mar 27, 2023

There is still something going on with Jackson versions with pekko-serialization-jackson and the cassandra driver pulled in from pekko-persistence-cassandra fighting over Jackson 2.11 vs 2.12.

@jrudolph
Copy link
Contributor

I am not that familiar with Pekko projection, does it make sense to do this change outside of the context of this problem specifically (or in other words is not having the changes you are pointing out an oversight?)

I'm also not familiar. Though, the proposed change only changes the examples, so that should be no problem.

@mdedetrich
Copy link
Contributor Author

mdedetrich commented Mar 27, 2023

There is still something going on with Jackson versions with pekko-serialization-jackson and the cassandra driver pulled in from pekko-persistence-cassandra fighting over Jackson 2.11 vs 2.12.

Yeah this is one of the reasons why I wanted to avoid fixing the dependency resolution issue right now, I had an intuition its more complicated than it looks on the surface. Definitely makes sense to resolve this at some point but it can be properly be done later (I made an issue at #20).

@jrudolph
Copy link
Contributor

There is still something going on with Jackson versions with pekko-serialization-jackson and the cassandra driver pulled in from pekko-persistence-cassandra fighting over Jackson 2.11 vs 2.12.

I looked into it and basically the issue seems to be that this setup only works with an older alpakka-cassandra version which used to use the cassandra 4.6.2 driver but now uses 4.13.0.

Yeah this is one of the reasons why I wanted to avoid fixing the dependency resolution issue right now, I had an intuition its more complicated than it looks on the surface. Definitely makes sense to resolve this at some point but it can be properly be done later (I will make an issue).

What does postponing mean exactly? For me it looks like, our current set of source code is just not compatible enough to allow pekko-projection to be used with pekko-persistence-cassandra. This might be a quite tricky issue that needs to be considered with all pekko modules in mind. Let's continue a discussion on a greater scale in apache/pekko#7.

@mdedetrich
Copy link
Contributor Author

What does postponing mean exactly? For me it looks like, our current set of source code is just not compatible enough to allow pekko-projection to be used with pekko-persistence-cassandra. This might be a quite tricky issue that needs to be considered with all pekko modules in mind. Let's continue a discussion on a greater scale in apache/incubator-pekko#7.

What I mean by postponing is that right now I am just interested in doing the package/conf renaming and later on updating header files/setting up for publishing etc etc. It may be true that the source code is not compatible, if so it should be done after this PR because its this PR in the first place that is changing the dependencies from Akka to Pekko.

@jrudolph
Copy link
Contributor

Sounds good for me!

Examples.akkaClusterShardingTyped,
Examples.akkaPersistenceCassandra,
Examples.akkaPersistenceJdbc,
Examples.akkaSerializationJackson,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jrudolph So I ended up having to remove Examples.pekkoSerializationJackson from here because

[error] Caused by: com.fasterxml.jackson.databind.JsonMappingException: Scala module 2.11.4 requires Jackson Databind version >= 2.11.0 and < 2.12.0
[error] 	at com.fasterxml.jackson.module.scala.JacksonModule.setupModule(JacksonModule.scala:61)
[error] 	at com.fasterxml.jackson.module.scala.JacksonModule.setupModule$(JacksonModule.scala:46)

Even through what I can tell the whole point of Examples.pekkoSerializationJackson is to actually force the version of jackson (which is 2.11.4) but it actually seems to be causing problems.

@mdedetrich mdedetrich marked this pull request as ready for review March 27, 2023 11:48
@nvollmar
Copy link

I couldn't leave comments on the changes for some reason. Found some @apidoc[pekko. that should be @apidoc[org.apache.pekko. since they are fqcn

1 similar comment
@nvollmar
Copy link

I couldn't leave comments on the changes for some reason. Found some @apidoc[pekko. that should be @apidoc[org.apache.pekko. since they are fqcn

@mdedetrich
Copy link
Contributor Author

mdedetrich commented Mar 27, 2023

I couldn't leave comments on the changes for some reason. Found some @apidoc[pekko. that should be @apidoc[org.apache.pekko. since they are fqcn

Faurly sure apidoc doesn't need FQCN because it refers to paradox root setting that has been set to org.apache.pekko. This is the same as other pekko projects and if it was wrong the doc/paradox project wouldn't even compile.

@nvollmar
Copy link

@mdedetrich thanks, good to know.

Copy link

@nvollmar nvollmar left a comment

Choose a reason for hiding this comment

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

lgtm

@mdedetrich mdedetrich merged commit e91a6db into apache:main Mar 27, 2023
@mdedetrich mdedetrich deleted the rename-package-to-pekko branch March 27, 2023 18:42
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.

None yet

3 participants