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

Fix Command line interface commands and connection management service to prompt for provider installation #25056

Merged
merged 6 commits into from
Nov 30, 2023

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Nov 28, 2023

Fixes #25050 and fixes #25055

Tested both commands from CLI args when working with unknown/known providers:

connect-default
openConnection-default
Note: Prompt added in Connection Management Service asks users to install extension again when user tries to connect despite canceling the first installation prompt.
cli-fixed

@coveralls
Copy link

coveralls commented Nov 28, 2023

Pull Request Test Coverage Report for Build 7039992431

  • 24 of 30 (80.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 41.594%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/sql/workbench/services/connection/browser/connectionDialogService.ts 5 6 83.33%
src/sql/workbench/contrib/commandLine/electron-sandbox/commandLine.ts 19 24 79.17%
Totals Coverage Status
Change from base Build 7038348309: 0.03%
Covered Lines: 30810
Relevant Lines: 69389

💛 - Coveralls

Copy link
Contributor

@Charles-Gagnon Charles-Gagnon left a comment

Choose a reason for hiding this comment

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

Looks good, aside from making sure the help stuff/comments are updated.

I'd also suggest making sure to validate that the SSMS New Notebook action still works as expected after the updates.

@cheenamalhotra
Copy link
Member Author

cheenamalhotra commented Nov 30, 2023

Looks good, aside from making sure the help stuff/comments are updated.

I'd also suggest making sure to validate that the SSMS New Notebook action still works as expected after the updates.

New Notebook action is fine, but other commands are currently not able to consume connection profile passed by user, due to #25068 - let me know your thoughts on it. I was thinking to deprecate the ConnectedContext and use the same BaseActionContext to keep things simplified.

@cheenamalhotra cheenamalhotra merged commit 6c1ba6e into main Nov 30, 2023
12 checks passed
@cheenamalhotra cheenamalhotra deleted the cheena/fix-openConnectionDialog branch November 30, 2023 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants