Skip to content

config revisions, all tests passing #166

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

Merged
merged 2 commits into from
Oct 16, 2018
Merged

config revisions, all tests passing #166

merged 2 commits into from
Oct 16, 2018

Conversation

MV10
Copy link
Contributor

@MV10 MV10 commented Oct 13, 2018

This PR is larger than I'd intended and I'm guessing it won't get reviewed, but perhaps the notes will generate some discussion/attention so I'll wait awhile before merging.

The primary goal was to simplify and improve consistency of configuration because:

  • this is one of the most complicated sinks to configure
  • use of the .NET DataColumn class and .NET System types was confusing and limiting
  • inconsistencies mapping between SQL types and System types
  • inconsistent type mapping behavior between .NET Framework and .NET Standard
  • too much special-case code for Standard Columns as a side-effect of original implementation

High-level change list:

Unifying column representation (Standard Columns and custom columns) results in this PR touching most of the code base. It retains full backwards compatibility with existing config-by-code or external-config options, but it turned out to be pretty easy to address nearly every open feature request as a byproduct of what changed anyway.

Issues:

Although it adds new functionality, I argue this is actually a simplification because there are fewer arbitrary restrictions. Normal SQL types and features are simply available and working out-of-the-box now. There is less to know, less to learn, and less you can get wrong now.

Major features / changes:

  • renamed CommonColumnOptions to SqlColumn (since it is now exposed)
  • Standard Column subclasses override properties like DataType to restrict options
  • AdditionalColumns (SqlColumn-based) replaces AdditionalDataColumns (DataColumn-based)
  • AdditionalDataColumns deprecated with Obsolete attribute
  • use of DataColumn class is now completely internal
  • Standard Column Id.BigInt obsolete, set DataType = SqlDbType.BigInt instead
  • internal DataColumns have ExtendedProperty ref to original SqlColumn
  • table creation completely rewritten
  • all SQL column data type handling works the same way everywhere now
  • SelfLog warnings for use of deprecated features
  • more throws for invalid option combinations, or SelfLog warnings if auto-corrected
  • eliminated hard-coded Standard Column names, smarter ColumnName property
  • ColumnOptions.PrimaryKey added (defaults to Id if included in Store collection)
  • ColumnOptions.ClusteredColumnstoreIndex property added
  • new unit tests, revised older tests for new SqlColumn support
  • SQL column data type parsing now case-insensitive (including deprecated config)
  • removed SQL binary/varbinary support, Serilog converts to truncated ScalarValue string
  • eliminated several class files
  • split ColumnOptions class into partial class files

Changes specific to .NET Standard configuration:

  • Standard Column config has full SqlColumn support
  • "id.bigInt" property deprecated, use "dataType":"bigInt"
  • CCI via "clusteredColumnstoreIndex" property
  • PK via "primaryKeyColumnName"

Changes specific to .NET Framework configuration:

  • sync'd supported types with .NET Standard list
  • internally config uses AdditionalColumns and SqlColumn instead of DataColumn
  • moved all but the main config section class to the SQL sink namespace
  • added central config method for new PR to expand appconfig support

@MV10
Copy link
Contributor Author

MV10 commented Oct 13, 2018

@MV10
Copy link
Contributor Author

MV10 commented Oct 16, 2018

Merging so I can PR the .NET Framework XML config changes...

@MV10 MV10 merged commit 2d1fa1b into serilog-mssql:dev Oct 16, 2018
@MV10 MV10 deleted the improve_config branch October 16, 2018 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment