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

Akka.Persistence.SqlServer plugin #842

Merged
merged 1 commit into from
Apr 24, 2015

Conversation

Horusiath
Copy link
Contributor

This PR introduces plugin with support for persistent Journal and Snapshot Store backed by SqlServer database.

  • Setup and configuration details are available in README.md. Public API is commented.
  • Test cases by default assume that test database (named: AkkaPersistenceSqlServerSpecDb) already exists on .\SQL-EXPRESS server Tests uses AkkaPersistenceSqlServerSpecDb.mdf database file located in Akka.Persistence.SqlServer.Tests/Resources directory.
  • Also Akka.Persistence.TestKit base specs in form of JournalSpec and SnapshotStoreSpec has been changed to require to run Initialize() method at the end of spec constructor in order to properly initialize the test cases [BREAKING CHANGE].

cc #667

table-name = SnapshotStore
schema-name = dbo
auto-initialize = on
connection-string = ""Data Source=.\\SQLEXPRESS;Initial Catalog=AkkaPersistenceSqlServerSpecDb;Integrated Security=True""
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be configured somehow. we can't expect the build serer and users to have this specific setup. see HOCON comment below.

@rogeralsing
Copy link
Contributor

IMO, just to be on the safe side, we should escape schema and table names when building queries.
This might be the correct way SqlCommandBuilder.QuoteIdentifier : https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlcommandbuilder.quoteidentifier%28v=vs.110%29.aspx

@rogeralsing
Copy link
Contributor

As for configuration settings on connection strings, I guess we should complete the substitution support in HOCON.

Maybe we can keep the current setup until we have the hocon bits in place

@Aaronontheweb
Copy link
Member

.SLN file currently has an MSBUILD failure according to TeamCity - need to update PR accordingly

@jcwrequests
Copy link

@Horusiath @rogeralsing Do you think it makes sense to also provide guidance on protecting the app config file? Whenever I deploy anything that has something sensitive like connection strings I always encrypted the config. I know other frameworks don't bother since it most shops that's an IT responsibility but that is not always the case. For the last 15 plus years I have been not only responsible for development but operations end and am alway concern about security. I am just sharing my thoughts. Once this has settled down I would love to see if that same thing could be done with Postgres and Cassandra.

@Aaronontheweb
Copy link
Member

Build Server can't run MSBuild at the moment, according to the error log. Would you mind taking a look at the .SLn @Horusiath ?

@Horusiath
Copy link
Contributor Author

@Aaronontheweb If fixed a sln file and build.cmd didn't notice any problems, so I guest it should be ok now.

@rogeralsing
Copy link
Contributor

Can this be merged or do we have to wait for sql support to be installed on the build server?

@Aaronontheweb
Copy link
Member

@rogeralsing I need some instructions from @Horusiath on how to update the build agents first

sqlexpress based test database

working SqlServerJournal, intializes SqlServerSnapshotStore

initial impl of SqlServer snapshot store

fixed rest of broken sql server snapshot store specs

Akka.Persistence.SqlServer comments for public API

final notest and docs for Akka.Persistence.SqlServer

safe escape of SQL string formats + mdf based tests

fixed sln file

added Akka.Persistence.SqlServer ldf test database file

modified FAKE script to skip Akka.Persistence.SqlServer on LocalDB not installed

separate build task for sql server tests
@Horusiath
Copy link
Contributor Author

As discussed with @Aaronontheweb I've created a separate build task (RunSqlServerTests) in FAKE for Akka.Persistence.SqlServer. It was possible to automatically recognize if Local DB is installed on host but it could potentially cause some problems on Linux. So right now it's a explicit choice of the user running a build script.

cc #891

@Aaronontheweb
Copy link
Member

looking good buddy - looking forward to seeing this hit the airwaves in v1.0.1

Aaronontheweb added a commit that referenced this pull request Apr 24, 2015
@Aaronontheweb Aaronontheweb merged commit 2a8da4c into akkadotnet:dev Apr 24, 2015
@Aaronontheweb
Copy link
Member

@Horusiath one minor thing - I believe we need a .nuspec file for this :p

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.

4 participants