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

Add a command line interface for connecting to a SQL Server #3047

Merged
merged 10 commits into from Oct 31, 2018

Conversation

Projects
None yet
2 participants
@shueybubbles
Contributor

shueybubbles commented Oct 30, 2018

This change adds server, database, user, and integrated command line flags similar to what mssql-cli supports for opening a connection to SQL server on ADS startup. If an existing connection is found in the root server group matching the criteria, ADS reuses it, otherwise it tries to create a new one. If the connection succeeds a new query editor window will open for that connection. Otherwise ADS will show the connection dialog to prompt for proper credentials.

@shueybubbles shueybubbles requested review from kburtram and anthonydresser Oct 30, 2018

@shueybubbles shueybubbles requested a review from MattIrv Oct 30, 2018

@kburtram

Overall looks good to me. But please address some of the comments in the review prior to merging...particularly regarding changes to files in vs directory.

Show resolved Hide resolved .vscode/launch.json Outdated
)
{
let profile = null;
if (this._environmentService && this._environmentService.args.server)

This comment has been minimized.

@kburtram

kburtram Oct 31, 2018

Member

_environmentService.args will never be undefined?

This comment has been minimized.

@shueybubbles

shueybubbles Oct 31, 2018

Contributor

the _args parameter to the EnvironmentService constructor is not optional and the EnvironmentService itself never checks it for undefined/null.

{
profile = new ConnectionProfile(_capabilitiesService, null);
// We want connection store to use any matching password it finds
profile.savePassword = true;

This comment has been minimized.

@kburtram

kburtram Oct 31, 2018

Member

is it the case we always want to save the password? there are some security concerns here (particularly on Linux where we don't have an encryption library and depend only on filesystem ACLs)

This comment has been minimized.

@shueybubbles

shueybubbles Oct 31, 2018

Contributor

ConnectionStore only looks for saved credentials that match the profile if savePassword is true. Also, we never pass any password on the command line. So if there's no saved credential that matches the server name/user name/database name combination, this connection attempt will fail, and the user will be shown the connection dialog. At that point they have the option to check/uncheck the Remember password checkbox.

profile = new ConnectionProfile(_capabilitiesService, null);
// We want connection store to use any matching password it finds
profile.savePassword = true;
profile.providerName = Constants.mssqlProviderName;

This comment has been minimized.

@kburtram

kburtram Oct 31, 2018

Member

this mechanism will only work for MSSQL connections?

This comment has been minimized.

@shueybubbles

shueybubbles Oct 31, 2018

Contributor

yes, currently. I assume we'd need to add more switches to support more server types in the future.

Show resolved Hide resolved src/sql/parts/commandLine/common/commandLineService.ts Outdated
Show resolved Hide resolved src/sql/parts/connection/common/connectionManagementService.ts Outdated
Show resolved Hide resolved src/vs/platform/environment/common/environment.ts Outdated
Show resolved Hide resolved src/vs/platform/environment/node/argv.ts Outdated
Show resolved Hide resolved src/vs/workbench/electron-browser/workbench.ts
@shueybubbles

This comment has been minimized.

Contributor

shueybubbles commented Oct 31, 2018

Couple questions - is there external documentation I should update with the new switches?
Is it a known issue that "azuredatastudio --help" isn't currently working? If not I'll open one.

shueybubbles added some commits Oct 31, 2018

@shueybubbles shueybubbles merged commit 7f66087 into master Oct 31, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details

@shueybubbles shueybubbles deleted the adsssms branch Oct 31, 2018

@kburtram

This comment has been minimized.

Member

kburtram commented Oct 31, 2018

@shueybubbles I don't think we have documentation on command-line switches, though I know CSS has asked for this. Please log issues for any gaps here. Thanks!

kburtram added a commit that referenced this pull request Oct 31, 2018

Add a command line interface for connecting to a SQL Server (#3047)
* Add switches for server, database, user, integrated auth

* Refactor into new commandline service

* Open query editor when passed server on command line

* Add tests

kburtram added a commit that referenced this pull request Oct 31, 2018

kburtram added a commit that referenced this pull request Nov 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment