Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.88%. Comparing base(b8e0c9f) to head(4c73ef8).
Report is 3 commits behind head on main.

@@            Coverage Diff             @@##             main    #3084      +/-   ##==========================================+ Coverage   72.70%   72.88%   +0.18%==========================================  Files         283      283                Lines       58975    58978       +3     ==========================================+ Hits        42875    42984     +109+ Misses      16100    15994     -106

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

@edwardneal There are a number of feature differences between the many engine variations this driver supports or happen to work with - is it really the responsibility of the driver to document this, or is the page you are linking to not enough?

I think the page I linked to would cover most cases, I'm thinking primarily about a once-over to make sure that the SqlClient documentation doesn't contradict it and is clear enough for people to understand what functionality the feature depends upon. The example I had in mind wasSqlBulkCopy - it might not be obvious that the lack of BULK INSERT support blocks this feature. I think that's probably the root cause of#2995.

#2995 refers to Fabric warehouse, which is something completely different from Fabric SQL Database

Sorry, slightly delayed here!

You're right, but the root cause is similar: some of SqlClient's functionality can't be supported on a particular engine variation, because that functionality relies upon a feature the engine doesn't support (in this case, SqlBulkCopy relies upon the BULK INSERT statement, which neither Fabric Warehouse nor Fabric SQL Database support.)

My broader point is that we should make sure that the SqlClient documentation for functionality which depends on BULK INSERT or Always Encrypted doesn't claim to be compatible across all engine variations, and that specifically for SqlBulkCopy we should make it clear that the feature depends upon server-side BULK INSERT support (since that's hard to discover in the documentation, and without that insight it's hard to know what won't support the feature.) If you want to keep this PR scoped specifically to the endpoint detection, I'm happy enough to check that documentation in a separate PR.

Sounds to me like a separate issue and PR would be suitable to prevent scope creep.

@mdaigle any thoughts on this?

@ErikEJ do review and update docs where the current endpoints are mentioned. Thanks!

@cheenamalhotra@mdaigle Docs updated and PR feedback addressed

@cheenamalhotra@mdaigle CI never kicked in??

I used the new resource designer in VS for the removal, please make sure to review the Designer file changes.

/azp run

I will revert the Designer changes..

@mdaigle Designer changes reverted

@mdaigle Designer changes reverted

I liked the Designer changes - mostly fixing typos from what I could see. Did this cause problems elsewhere?

Also, you seem to have rebasedand pushed 1 new commit to revert the Designer changes, but GitHub still has me marked as Approved. I haven't reviewed the rebase nor the 1 new commit, so that's weird.

@paulmedynski The designer changes caused the build to fail.

Lets try to avoid scope creep, I am happy to submit a new PR with the designer changes next. (And yes, it looked like mostly typos)

@paulmedynski The PR pipeline was not run 😢 - hope I did not break main 🙏

Sigh, I will monitor. Looks good so far though.