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

setQueryExecutionOptions vs setExecutionOptions #6721

Closed
aaomidi opened this issue Aug 13, 2019 · 2 comments
Closed

setQueryExecutionOptions vs setExecutionOptions #6721

aaomidi opened this issue Aug 13, 2019 · 2 comments
Assignees
Labels
Area - Extensibility Enhancement Request for new features or functionality
Milestone

Comments

@aaomidi
Copy link
Contributor

aaomidi commented Aug 13, 2019

In the API, we currently have two ways to set the execution options for queries.

First one is through setQueryExecutionOptions for a whole connection. This method has the following signature:

setQueryExecutionOptions(ownerUri: string, options: QueryExecutionOptions): Thenable<void>;

Where QueryExecutionOptions is:

export interface QueryExecutionOptions {
	options: Map<string, any>;
}

Meanwhile, setExecutionOptions has the following signature:

setExecutionOptions(options: Map<string, any>): Thenable<void>;

Proposal: Change the QueryExecutionOptions interface to:

export type QueryExecutionOptions = Map<String, any>;

And change setExecutionOptions to:

setExecutionOptions(options: Map<string, any> | QueryExecutionOptions): Thenable<void>;

@kburtram
Copy link
Member

These APIs are for two different purposes. The first API is part of the QueryProvider interface, which should be implemented by extensions that want to add a new Data Provider type (such as MSSQL, Postgres, MariaDB, etc). The second API is part of the ADS Extensibility API that allows general extensions to update the options for a particular editor. Extensions that aren't adding new Data Providers should only use setExecutionOptions.

Perhaps it makes sense to move the Provider API definitions out of the azdata file to reduce confusion. For 3rd-party extensions I think the current definition of setExecutionOptions is pretty intuitive, just pass in a Map of options. I'm not sure if there is value in introducing the QueryExecutionOptions alias in that use-case.

@kburtram kburtram added the Enhancement Request for new features or functionality label Aug 13, 2019
@kburtram kburtram added this to the Backlog milestone Aug 13, 2019
@kburtram
Copy link
Member

Closing this older API-related enhancement to focus on customer scenario-focused bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area - Extensibility Enhancement Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

2 participants