pjfanning commented on code in PR #363:
URL:
https://github.com/apache/pekko-persistence-jdbc/pull/363#discussion_r2520023457
##########
core/src/main/resources/schema/mysql/mysql-create-schema.sql:
##########
@@ -36,3 +36,19 @@ CREATE TABLE IF NOT EXISTS snapshot (
meta_ser_manifest TEXT,
meta_payload BLOB,
PRIMARY KEY (persistence_id, sequence_number));
+
+CREATE TABLE IF NOT EXISTS durable_state
+(
+ global_offset BIGINT AUTO_INCREMENT,
+ persistence_id VARCHAR(255) NOT NULL,
+ revision BIGINT NOT NULL,
+ state_payload LONGBLOB NOT NULL,
+ state_serial_id INTEGER NOT NULL,
+ state_serial_manifest VARCHAR(255),
+ tag VARCHAR(255),
+ state_timestamp BIGINT NOT NULL,
+ PRIMARY KEY (persistence_id),
+ UNIQUE KEY state_global_offset_uk (global_offset)
+);
+CREATE INDEX state_tag_idx on durable_state (tag);
+CREATE INDEX state_global_offset_idx on durable_state (global_offset);
Review Comment:
[nit] add a newline at end of file
##########
core/src/test/scala/org/apache/pekko/persistence/jdbc/state/scaladsl/StateSpecBase.scala:
##########
@@ -47,13 +47,11 @@ abstract class StateSpecBase(val config: Config,
schemaType: SchemaType)
implicit lazy val e: ExecutionContext = system.dispatcher
private[jdbc] def schemaTypeToProfile(s: SchemaType) = s match {
- case H2 => slick.jdbc.H2Profile
- case Postgres => slick.jdbc.PostgresProfile
- // TODO https://github.com/apache/pekko-persistence-jdbc/issues/174
- // case MySQL => slick.jdbc.MySQLProfile
+ case H2 => slick.jdbc.H2Profile
+ case Postgres => slick.jdbc.PostgresProfile
+ case MySQL => slick.jdbc.MySQLProfile
case SqlServer => slick.jdbc.SQLServerProfile
case Oracle => slick.jdbc.OracleProfile
- case _ => throw new UnsupportedOperationException(s"Unsupported
<$s> for durableState.")
Review Comment:
keep this - it causes no harm and is useful if we add a new db type and
forget to support it here
##########
core/src/test/scala/org/apache/pekko/persistence/jdbc/state/scaladsl/JdbcDurableStateSpec.scala:
##########
@@ -83,9 +83,8 @@ abstract class JdbcDurableStateSpec(config: Config,
schemaType: SchemaType) exte
e shouldBe
an[org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException]
case Postgres =>
e shouldBe an[org.postgresql.util.PSQLException]
- // TODO https://github.com/apache/pekko-persistence-jdbc/issues/174
- // case MySQL =>
- // e shouldBe an[java.sql.SQLIntegrityConstraintViolationException]
+ case MySQL =>
+ e shouldBe an[java.sql.SQLIntegrityConstraintViolationException]
Review Comment:
'a' not 'an' before 'j'
##########
core/src/test/scala/org/apache/pekko/persistence/jdbc/state/scaladsl/StateSpecBase.scala:
##########
@@ -109,11 +107,10 @@ abstract class StateSpecBase(val config: Config,
schemaType: SchemaType)
f(system)
} finally {
system.actorSelection("system/" +
"pekko-persistence-jdbc-durable-state-sequence-actor").resolveOne().onComplete {
- case Success(actorRef) => {
+ case Success(actorRef) =>
Review Comment:
I'm not a big fan of reformatting existing code that is not related to the PR
##########
integration-test/src/test/scala/org/apache/pekko/persistence/jdbc/integration/MySQLDurableStateStorePluginSpec.scala:
##########
@@ -0,0 +1,21 @@
+/*
Review Comment:
new files should use the standard Apache header unless there are methods
copied in from existing files - where we should use that file's header
eg
https://github.com/apache/pekko-persistence-jdbc/blob/main/project/PekkoCoreDependency.scala#L1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
