-
Notifications
You must be signed in to change notification settings - Fork 285
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
Feature | Add SqlDataSourceEnumerator #1430
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added UDP Broadcast to SSRP
Kaur-Parminder
added
the
Area\Managed SNI
Issues that are targeted to the Managed SNI codebase.
label
Dec 13, 2021
…Native and Managed) Added SqlDataSourceEnumerator Public API and AppContext for Helpers (Native and Managed)
Kaur-Parminder
changed the title
Managed Implementation for SqlDataSourceEnumerator: Phase 1
Implementation for SqlDataSourceEnumerator: Managed, Native
Dec 21, 2021
Added AppContext change to unit test
DavoudEshtehari
added
the
Area\Native SNI
Issues that are targeted to the Native SNI codebase.
label
Feb 3, 2022
DavoudEshtehari
changed the title
Implementation for SqlDataSourceEnumerator: Managed, Native
Feature | Add SqlDataSourceEnumerator
Feb 3, 2022
…to avoid heap corruption issue
Kaur-Parminder
force-pushed
the
MSDEnumerator
branch
from
February 7, 2022 19:46
5b6e9b9
to
0bc15f8
Compare
# Conflicts: # src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs # src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj
Kaur-Parminder
added
🆕 Public API
Issues/PRs that introduce new APIs to the driver.
and removed
Area\Managed SNI
Issues that are targeted to the Managed SNI codebase.
Area\Native SNI
Issues that are targeted to the Native SNI codebase.
labels
Feb 7, 2022
…a from SQL Browser
Kaur-Parminder
requested review from
DavoudEshtehari,
johnnypham and
JRahnama
February 7, 2022 20:28
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.
For the record:
- Sanitize your code by considering the warning and messages in the Error List.
- Include traces using
EventSource
as much as possible? - Add documentation for the public APIs.
- Include the new public APIs in ref files.
- Add inline comments with reference.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlDataSourceEnumeratorTest.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Sql/SqlDataSourceEnumerator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Sql/SqlDataSourceEnumerator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Sql/SqlDataSourceEnumerator.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Sql/SqlDataSourceEnumeratorManagedHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Sql/SqlDataSourceEnumeratorNativeHelper.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlDataSourceEnumeratorTest.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SSRP.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlDataSourceEnumeratorTest.cs
Outdated
Show resolved
Hide resolved
exclude appcontext test
Comments and url references
review comment
fix strbuilder overwrite
VS Suggestions
body expression for GetTimeoutSeconds
created util for sqldatasourceenumerator
more utils
license for util file
JRahnama
approved these changes
Mar 2, 2022
DavoudEshtehari
suggested changes
Mar 4, 2022
Comment on lines
28
to
38
[DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIServerEnumOpenWrapper")] | ||
internal static extern IntPtr SNIServerEnumOpen(); | ||
|
||
[DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIServerEnumCloseWrapper")] | ||
internal static extern void SNIServerEnumClose([In] IntPtr packet); | ||
|
||
[DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIServerEnumReadWrapper", CharSet = CharSet.Unicode)] | ||
internal static extern int SNIServerEnumRead([In] IntPtr packet, | ||
[In][MarshalAs(UnmanagedType.LPArray)] char[] readBuffer, | ||
[In] int bufferLength, | ||
[MarshalAs(UnmanagedType.Bool)] out bool more); |
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.
These lines should be moved into SNINativeMethodWrapper.Windows.cs
. And modify the native helper accordingly.
SNI Update
This reverts commit 7aebf45.
Merged
SqlDataSourceEnumerator test update and different file for unix, windows
johnnypham
reviewed
Mar 8, 2022
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Sql/SqlDataSourceEnumeratorManagedHelper.cs
Outdated
Show resolved
Hide resolved
johnnypham
reviewed
Mar 9, 2022
...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj
Outdated
Show resolved
Hide resolved
DavoudEshtehari
approved these changes
Mar 9, 2022
johnnypham
approved these changes
Mar 9, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added UDP Broadcast to SSRP
Details about Request/Response/timeouts are available in specification
Update: