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

Update PersistentEntityRDBMS.md #1681

Closed
wants to merge 1 commit into
base: 1.4.x
from

Conversation

Projects
None yet
4 participants
@yash-bhardwaj
Copy link

yash-bhardwaj commented Dec 19, 2018

As per com.lightbend.lagom.internal.persistence.jdbc.SlickProvider.scala:52 in "com.lightbend.lagom" % "lagom-sbt-plugin" % "1.4.9":

private val readSideConfig = system.settings.config.getConfig("lagom.persistence.read-side.jdbc")
  private val jdbcConfig = system.settings.config.getConfig("lagom.persistence.jdbc")
  private val createTables = jdbcConfig.getConfig("create-tables")
  val autoCreateTables: Boolean = createTables.getBoolean("auto")

  // users can disable the usage of jndiDbName for userland read-side operations by
  // setting the jndiDbName to null. In which case we fallback to slick.db.
  // slick.db must be defined otherwise the application will fail to start
  val db = {
    if (readSideConfig.hasPath("slick.jndiDbName")) {
      new InitialContext()
        .lookup(readSideConfig.getString("slick.jndiDbName"))
        .asInstanceOf[Database]
    } else if (readSideConfig.hasPath("slick.db")) {
      Database.forConfig("slick.db", readSideConfig)
    } else {
      throw new RuntimeException("Cannot start because read-side database configuration is missing. " +
        "You must define either 'lagom.persistence.read-side.jdbc.slick.jndiDbName' or 'lagom.persistence.read-side.jdbc.slick.db' in your application.conf.")
    }
  }

  val profile = DatabaseConfig.forConfig[JdbcProfile]("slick", readSideConfig).profile

It is reading slick.profile from lagom.persistence.read-side.jdbc.slick.profile, not from jdbc-defaults.slick.driver = "slick.driver.PostgresDriver$"

Pull Request Checklist

  • Have you read through the contributor guidelines?
  • Have you signed the Lightbend CLA?
  • Have you added copyright headers to new files?
  • Have you updated the documentation?
  • Have you added tests for any changed functionality?

Fixes

Fixes #1672

Purpose

What does this PR do?

Background Context

Why did you take this approach?

References

#1672
Are there any relevant issues / PRs / mailing lists discussions?

Update PersistentEntityRDBMS.md
As per code:
```

private val readSideConfig = system.settings.config.getConfig("lagom.persistence.read-side.jdbc")
  private val jdbcConfig = system.settings.config.getConfig("lagom.persistence.jdbc")
  private val createTables = jdbcConfig.getConfig("create-tables")
  val autoCreateTables: Boolean = createTables.getBoolean("auto")

  // users can disable the usage of jndiDbName for userland read-side operations by
  // setting the jndiDbName to null. In which case we fallback to slick.db.
  // slick.db must be defined otherwise the application will fail to start
  val db = {
    if (readSideConfig.hasPath("slick.jndiDbName")) {
      new InitialContext()
        .lookup(readSideConfig.getString("slick.jndiDbName"))
        .asInstanceOf[Database]
    } else if (readSideConfig.hasPath("slick.db")) {
      Database.forConfig("slick.db", readSideConfig)
    } else {
      throw new RuntimeException("Cannot start because read-side database configuration is missing. " +
        "You must define either 'lagom.persistence.read-side.jdbc.slick.jndiDbName' or 'lagom.persistence.read-side.jdbc.slick.db' in your application.conf.")
    }
  }

  val profile = DatabaseConfig.forConfig[JdbcProfile]("slick", readSideConfig).profile
```

It is reading slick.profile from `lagom.persistence.read-side.jdbc.slick.profile`, not from `jdbc-defaults.slick.driver = "slick.driver.PostgresDriver$"`
@lightbend-cla-validator

This comment has been minimized.

Copy link

lightbend-cla-validator commented Dec 19, 2018

Hi @yash-bhardwaj,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@yash-bhardwaj

This comment has been minimized.

Copy link

yash-bhardwaj commented Dec 19, 2018

I did sign the CLA.

@@ -48,7 +48,7 @@ db.default {
url = "jdbc:postgresql://database.example.com/lagom-db"
}
jdbc-defaults.slick.profile = "slick.jdbc.PostgresProfile$"
lagom.persistence.read-side.jdbc.slick.profile = "slick.jdbc.PostgresProfile$"

This comment has been minimized.

@TimMoore

TimMoore Dec 20, 2018

Member

This doesn't look right to me. This configuration would only configure the profile for the read-side processors, not for the persistent entity.

The defaults are configured so that the original property will work, and jdbc-defaults.slick.profile will be used for both write-side and read-side unless you explicitly override one or the other:

# The Slick profile
profile = ${?jdbc-defaults.slick.profile}

Is there a specific problem that you had?

@renatocaval

This comment has been minimized.

Copy link
Member

renatocaval commented Dec 20, 2018

@yash-bhardwaj, the documentation is correct as explained by @TimMoore.

We need to configure jdbc-defaults.slick.profile. This property is then used to configure the plugin components in akka-persistence-jdbc and the read-side.

Therefore, I'm closing this PR. I see that you reported it here: https://discuss.lightbend.com/t/how-to-bind-slick-dependency-with-lagom/2935/3

I understand that if you dig into the code you don't immediately see that you are reading a property that is being set by another, but that's necessary otherwise users will need to configure the profile 4 times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment