Skip to content

Conversation

@CESARDELATORRE
Copy link
Contributor

@CESARDELATORRE CESARDELATORRE commented Aug 21, 2019

DatabaseLoader sample loading an IDataView from SQL Server localdb

Using public preview.

@CESARDELATORRE CESARDELATORRE merged commit bc306e5 into dotnet:master Sep 2, 2019
Copy link

@divega divega left a comment

Choose a reason for hiding this comment

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

@CESARDELATORRE, I did a deferred review. The experience seems pretty good. And I agree with you that it could be even better with some sugar method that loads directly from the arguments of DatabaseSource.

I can also see other possible shortcuts of similar nature. For example, although DbProviderFactory is the all encompassing root concept if you need everything from an ADO.NET provider, a DbConnection can give you everything but DbConnectionStringBuilder, and it is a much more familiar abstraction for most users than the DbProviderFactory. So, unless you need to manipulate connection strings in a provider agnostic way (not commonly a useful thing to do), you could make the sugar Load method generic on the provider's DbConnection type.

All in all, I would would love to meet with you and the devs and walk trough the product code and API. It is likely that more things like this will come up.

Elizabethhanson pushed a commit to Elizabethhanson/machinelearning-samples that referenced this pull request Sep 10, 2021
…otnet#617)

* DatabaseLoader sample - Baseline

* DatabaseLoader working against localdb attached from DB path file.

* Removed unneeded-old project file

* Added tbd README.md file

* Removed internal Nuget feeds from the nuget.config

* Added a shorter name for the localdb database instead of based on the file path

* Added Retries logic policy to the Connection String when using Azure SQL Database

* Simplified so it doesnt use the ProviderFactory code but simple an SqlClientFactory.Instance

* Updated to public Previews

* Removed unneeded CsvParser reference

* Comments update doe conn-strings
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.

2 participants