Skip to content
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

Update connectivity-architecture-overview.md #9960

Open
wants to merge 3 commits into
base: live
Choose a base branch
from

Conversation

azarboon
Copy link
Contributor

@azarboon azarboon commented Dec 7, 2024

Connection pooling has been barely discussed in the documentation of SQL Managed Instance. Given the authority of this documentation, I've elaborated it to inform readers about the nuances.

Copy link
Contributor

@azarboon : Thanks for your contribution! The author(s) have been notified to review your proposed change.

Copy link
Contributor

Learn Build status updates of commit 4740126:

✅ Validation status: passed

File Status Preview URL Details
azure-sql/managed-instance/connectivity-architecture-overview.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@zoran-rilak-msft
Copy link
Contributor

@azarboon thank you for this very nicely written treatment of pooling!

I think it should be kicked up a floor and appear in shared SQL MI & SQL DB docs. Client-side pooling applies to both, after all :)
(Also it's not AAD any more, today we should call it Entra ID).

@MashaMSFT can you work your magic to move this article to the "shared concepts" space between SQL MI and SQL DB?
Thank you!

Copy link
Contributor

Learn Build status updates of commit 09cb232:

⚠️ Validation status: warnings

File Status Preview URL Details
azure-sql/managed-instance/connectivity-architecture-overview.md ⚠️Warning Details

azure-sql/managed-instance/connectivity-architecture-overview.md

  • Line 154, Column 385: [Warning: file-not-found - See documentation] Invalid file link: '../../../docs/framework/data/adonet/sql-server-connection-pooling.md'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

replaced the link with full path
Copy link
Contributor

Learn Build status updates of commit f336d66:

💡 Validation status: suggestions

File Status Preview URL Details
azure-sql/managed-instance/connectivity-architecture-overview.md 💡Suggestion Details

azure-sql/managed-instance/connectivity-architecture-overview.md

  • Line 154, Column 385: [Suggestion: docs-link-absolute - See documentation] Absolute link 'https://learn.microsoft.com/dotnet/framework/data/adonet/sql-server-connection-pooling' will be broken in isolated environments. Replace with a relative link.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

@azarboon
Copy link
Contributor Author

azarboon commented Dec 9, 2024

@zoran-rilak-msft thanks for suggestions. I've edited them. I couldn't get right the relative path to that ADO article so I added full URL. You should be able to edit the file by yourself (I enabled it). Kindly please sign it off as you deem appropriate.

@zoran-rilak-msft
Copy link
Contributor

zoran-rilak-msft commented Dec 9, 2024

Thanks @azarboon !

@MashaMSFT - I think the right address should be /dotnet/framework/data/adonet/sql-server-connection-pooling?
Also, let's see if this paragraph should go in the shared MI & DB section.

@ttorble
Copy link
Contributor

ttorble commented Dec 9, 2024

Thanks @zoran-rilak-msft

IMPORTANT: When the changes are ready for publication, adding a #sign-off comment is the best way to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator bot added the aq-pr-triaged tracking label for the PR review team label Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants