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

Change default config values to be more greenfield friendly #195

Merged

Conversation

Arkatufus
Copy link
Contributor

Fixes #194

Changes

  • akka.persistence.journal.sql
    • Change delete-compatibility-mode to false
    • Change prefer-parameters-on-multirow-insert to false
    • Change use-clone-connection to true
    • Change auto-initialize to true
  • akka.persistence.query.journal.sql
    • Change use-clone-connection to true
  • akka.persistence.snapshot-store.sql
    • Change use-clone-connection to true
    • Change auto-initialize to true
    • Remove compatibility-mode, it is not being used.

@@ -16,12 +16,12 @@
# If true, journal_metadata is created and used for deletes
# and max sequence number queries.
# note that there is a performance penalty for using this.
delete-compatibility-mode = true
delete-compatibility-mode = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change delete-compatibility-mode to false, we want to push the use of the new tag table for CQRS reasons

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@@ -48,7 +48,7 @@
# Linq2Db by default will use a built string for multi-row inserts
# Somewhat counterintuitively, this is faster than using parameters in most cases,
# But if you would prefer parameters, you can set this to true.
prefer-parameters-on-multirow-insert = true
prefer-parameters-on-multirow-insert = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change prefer-parameters-on-multirow-insert to false.
Use of better optimized path should be encouraged.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@@ -72,7 +72,7 @@

# Only set to TRUE if unit tests pass with the connection string you intend to use!
# This setting will go away once https://github.com/linq2db/linq2db/issues/2466 is resolved
use-clone-connection = false
use-clone-connection = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change use-clone-connection to true.
The original issue was closed with a temporary patch, it is better to leave this to true.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@@ -99,7 +99,7 @@
# should corresponding journal table be initialized automatically
# if delete-compatibility-mode is true, both tables are created
# if delete-compatibility-mode is false, only journal table will be created.
auto-initialize = off
auto-initialize = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Persistence plugins should work as is out of the box, database tables should be initialized automatically.
This is open for discussion, should this be reverted?

Copy link
Member

Choose a reason for hiding this comment

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

Nah, all other plugins work this way. LGTM.

@@ -354,7 +363,7 @@

# Only set to TRUE if unit tests pass with the connection string you intend to use!
# This setting will go away once https://github.com/linq2db/linq2db/issues/2466 is resolved
use-clone-connection = false
use-clone-connection = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change use-clone-connection to true.
The original issue was closed with a temporary patch, it is better to leave this to true.

@@ -15,7 +15,7 @@

# Only set to TRUE if unit tests pass with the connection string you intend to use!
# This setting will go away once https://github.com/linq2db/linq2db/issues/2466 is resolved
use-clone-connection = false
use-clone-connection = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change use-clone-connection to true.
The original issue was closed with a temporary patch, it is better to leave this to true.

@@ -30,10 +30,8 @@

dao = "Akka.Persistence.Sql.Snapshot.ByteArraySnapshotDao, Akka.Persistence.Sql"

compatibility-mode = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This setting was never used in code

# if true, tables will attempt to be created.
auto-initialize = false
auto-initialize = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Persistence plugins should work as is out of the box, database tables should be initialized automatically.
This is open for discussion, should this be reverted?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense to me, I start greenfield from an empty db

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

LGTM - we can gather data from users like @CumpsD and others who want to use this in new projects how well this all works

@@ -16,12 +16,12 @@
# If true, journal_metadata is created and used for deletes
# and max sequence number queries.
# note that there is a performance penalty for using this.
delete-compatibility-mode = true
delete-compatibility-mode = false
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@@ -48,7 +48,7 @@
# Linq2Db by default will use a built string for multi-row inserts
# Somewhat counterintuitively, this is faster than using parameters in most cases,
# But if you would prefer parameters, you can set this to true.
prefer-parameters-on-multirow-insert = true
prefer-parameters-on-multirow-insert = false
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@@ -72,7 +72,7 @@

# Only set to TRUE if unit tests pass with the connection string you intend to use!
# This setting will go away once https://github.com/linq2db/linq2db/issues/2466 is resolved
use-clone-connection = false
use-clone-connection = true
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@@ -99,7 +99,7 @@
# should corresponding journal table be initialized automatically
# if delete-compatibility-mode is true, both tables are created
# if delete-compatibility-mode is false, only journal table will be created.
auto-initialize = off
auto-initialize = true
Copy link
Member

Choose a reason for hiding this comment

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

Nah, all other plugins work this way. LGTM.

@Aaronontheweb Aaronontheweb merged commit 4ad062d into akkadotnet:dev Apr 3, 2023
@CumpsD
Copy link
Member

CumpsD commented Apr 5, 2023

LGTM - we can gather data from users like @CumpsD and others who want to use this in new projects how well this all works

Once the documentation is in, I will test it in a greenfield project :)

@CumpsD
Copy link
Member

CumpsD commented Apr 29, 2023

I migrated and it works fine out of the box 👍🏻

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.

Update default HOCON configuration to be greenfield project friendly
3 participants