intuit / QuickBooks-V3-DotNET-SDK

.Net SDK for QuickBooks REST API v3 services
Apache License 2.0
107 stars 138 forks source link

Please remove serilog dependencies #315

Open smasherprog opened 1 year ago

smasherprog commented 1 year ago

We have been bitten by this again in a recent release. This logging library should NOT be a dependency in your nuget package.

smasherprog commented 1 year ago

We are currently going into a freeze on updates to quickbooks library due to issues with serilog.

Server Error in '/' Application. Method not found: 'Serilog.LoggerConfiguration Serilog.FileLoggerConfigurationExtensions.File(Serilog.Configuration.LoggerSinkConfiguration, System.String, Serilog.Events.LogEventLevel, System.String, System.IFormatProvider, System.Nullable1<Int64>, Serilog.Core.LoggingLevelSwitch, Boolean, Boolean, System.Nullable1, Serilog.RollingInterval, Boolean, System.Nullable`1, System.Text.Encoding, Serilog.Sinks.File.FileLifecycleHooks)'.

smasherprog commented 1 year ago

Quickbooks is not adding version ranges to their nuget package for their dependencies. This has been going on for years now?

redhorseck commented 1 year ago

Yes. Years.

eswann commented 1 year ago

I don't understand why they took a hard dependency on Serilog (and a specific version at that). This is exactly why libraries should code against the M$ ILogger interface, even if it's less ideal than using Serilog directly. This has us unable to upgrade the SDK and we can't pin our version of Serilog to what they are using.

clement911 commented 1 year ago

Depending on your needs, you might want to consider using the community .NET SDK QuickBooksSharp. It has a much lighter footprint and a more modern API up to date with latest .NET practices. Disclaimer: I maintain this library.

redhorseck commented 1 year ago

Thanks I will check it out again. I had look at it but the last update was 9 months ago so I wasn't sure if it was still being maintained. Good to hear that it is.

eswann commented 1 year ago

@clement911 I would check it out, but for us there is already a lot of code using the existing SDK, and don't have the time (or permission) to rewrite all of it to use a different library.

attahameed commented 7 months ago

Is there any update on the request. It stopping us from using the latest version.

drusellers commented 6 months ago

Why is this library using Serilog rather than Microsoft Extensions Logging? If NOTHING else, this should be an optional dependency.

xosevilla commented 6 months ago

Still not fixed. What version of this 'Serilog.Sinks.File' dll is expected?

CyanideMonkey commented 4 months ago

I had some hope that with 14.6.3.7 dropping recently that we might be able to update Serilog.Sinks.Seq with it, but still stuck 😓

steveschmitt commented 3 days ago

I found a workaround that seems to be working for me. It is possible to inject a Serilog.ILogger into the Ipp classes so that it won't attempt to use obsolete methods to create one. I just used the static Serilog.Log.Logger, but you can create another Serilog.ILogger instance if needed.

I had to change my code in two places: where I create a ServiceContext, and where I create an OAuth2Client.

var serviceContext = new ServiceContext(realmId, IntuitServicesType.QBO, reqValidator, configurationProvider);
// added this line to inject logger
serviceContext.IppConfiguration.AdvancedLogger = new AdvancedLogger { 
    RequestAdvancedLog = new RequestAdvancedLog { CustomLogger = Serilog.Log.Logger } };

var oauthClient = new OAuth2Client(clientId, clientSecret, redirectUrl, environment) {
    // added this line to inject logger
    CustomLogger = Serilog.Log.Logger
}

Depending upon your use of the API, there may be more places that this sort of thing is needed.

Just FYI, I am using

smasherprog commented 3 days ago

Is this still really a thing and hasn't been fixed yet?

smasherprog commented 3 days ago

I honestly think this library is maintained by interns.