-
Notifications
You must be signed in to change notification settings - Fork 148
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
Add SqlInsertStatementWriter which uses INSERT statements instead of SqlBulkCopy #510
Add SqlInsertStatementWriter which uses INSERT statements instead of SqlBulkCopy #510
Conversation
Hi @BrettJaner! Thank you for the contribution. It is very welcome. Can you please update the documentation in README.md to relfect your changes? |
Hi @ckadluba, I've updated the README.md. |
After reviewing my code for possible breaking changes, I realized I missed the configuration implementation (eg. ability to specify |
src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertBatchWriter.cs
Outdated
Show resolved
Hide resolved
src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertBatchWriter.cs
Outdated
Show resolved
Hide resolved
src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertBatchWriter.cs
Outdated
Show resolved
Hide resolved
@BrettJaner Can you please add also one integration test using the new |
I've added integration tests to switch between the two batch writers (00cc7e9). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. The new SqlInsertStatementWriter class looks good. :)
Addresses issue (#509)
Summary of changes:
UseSqlBulkCopy
toMSSqlServerSinkOptions
defaulting totrue
UseSqlBulkCopy
from appsettings.jsonSqlInsertStatementWriter
which uses INSERT statements (highly based off of theSqlLogEventWriter
used by theMSSqlServerAuditSink
)ISqlBulkBatchWriter
andISqlLogEventWriter
SqlLogEventWriter
as it is now redundantSqlInsertStatementWriterTests