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(redshift): fix cache in connection wizard #3926

Merged
merged 3 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Redshift: The output message is not clear when a sql query is successful but returns no record."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Redshift: Re-launched connection wizard shows the old (stale) connection."
}
4 changes: 2 additions & 2 deletions src/redshift/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { RedshiftNotebookSerializer } from './notebook/redshiftNotebookSerialize
import { RedshiftNotebookController } from './notebook/redshiftNotebookController'
import { CellStatusBarItemProvider } from './notebook/cellStatusBarItemProvider'
import { Commands } from '../shared/vscode/commands2'
import { NotebookConnectionWizard } from './wizards/connectionWizard'
import { NotebookConnectionWizard, RedshiftNodeConnectionWizard } from './wizards/connectionWizard'
import { ConnectionParams, ConnectionType } from './models/models'
import { DefaultRedshiftClient } from '../shared/clients/redshiftClient'
import { localize } from '../shared/utilities/vsCodeUtils'
Expand Down Expand Up @@ -119,7 +119,7 @@ function getNotebookConnectClickedHandler(
function getEditConnectionHandler(outputChannel: vscode.OutputChannel) {
return async (redshiftWarehouseNode: RedshiftWarehouseNode) => {
try {
const connectionParams = await redshiftWarehouseNode.connectionWizard!.run()
const connectionParams = await new RedshiftNodeConnectionWizard(redshiftWarehouseNode).run()
if (connectionParams) {
if (connectionParams.connectionType === ConnectionType.DatabaseUser) {
const secretArnFetched =
Expand Down
6 changes: 2 additions & 4 deletions src/redshift/explorer/redshiftWarehouseNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,14 @@ export class RedshiftWarehouseNode extends AWSTreeNodeBase implements AWSResourc
constructor(
public readonly parent: RedshiftNode,
public readonly redshiftWarehouse: AWSResourceNode,
public readonly warehouseType: RedshiftWarehouseType,
public readonly connectionWizard?: RedshiftNodeConnectionWizard
public readonly warehouseType: RedshiftWarehouseType
) {
super(redshiftWarehouse.name, vscode.TreeItemCollapsibleState.Collapsed)
this.tooltip = redshiftWarehouse.name
this.contextValue = 'awsRedshiftWarehouseNode'
this.arn = redshiftWarehouse.arn
this.name = redshiftWarehouse.name
this.redshiftClient = parent.redshiftClient
this.connectionWizard = connectionWizard ?? new RedshiftNodeConnectionWizard(this)
const existingConnectionParams = getConnectionParamsState(this.arn)
if (existingConnectionParams && existingConnectionParams !== deleteConnection) {
this.connectionParams = existingConnectionParams as ConnectionParams
Expand Down Expand Up @@ -125,7 +123,7 @@ export class RedshiftWarehouseNode extends AWSTreeNodeBase implements AWSResourc
this.connectionParams = existingConnectionParams as ConnectionParams
} else {
// No connectionParams: trigger connection wizard to get user input
this.connectionParams = await this.connectionWizard!.run()
this.connectionParams = await new RedshiftNodeConnectionWizard(this).run()
if (!this.connectionParams) {
return this.getClickToEstablishConnectionNode()
}
Expand Down
2 changes: 1 addition & 1 deletion src/redshift/notebook/redshiftNotebookController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class RedshiftNotebookController {
records.push(...result.statementResultResponse.Records)
} else {
return new vscode.NotebookCellOutput([
vscode.NotebookCellOutputItem.text('No records.', 'text/plain'),
vscode.NotebookCellOutputItem.text('Query completed — No rows returned.', 'text/plain'),
])
}
} while (nextToken)
Expand Down
34 changes: 6 additions & 28 deletions src/redshift/wizards/connectionWizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,9 @@ import { SecretsManagerClient } from '../../shared/clients/secretsManagerClient'
import { redshiftHelpUrl } from '../../shared/constants'

export class RedshiftNodeConnectionWizard extends Wizard<ConnectionParams> {
public constructor(
node: RedshiftWarehouseNode,
connectionType?: ConnectionType,
database?: string,
username?: string,
password?: string,
secret?: string
) {
public constructor(node: RedshiftWarehouseNode) {
super({
initState: {
connectionType: connectionType ? connectionType : undefined,
database: database ? database : undefined,
username: username ? username : undefined,
password: password ? password : undefined,
secret: secret ? secret : undefined,
warehouseIdentifier: node.name,
warehouseType: node.warehouseType,
},
Expand All @@ -48,13 +36,13 @@ export class RedshiftNodeConnectionWizard extends Wizard<ConnectionParams> {
)

this.form.database.bindPrompter(getDatabasePrompter, {
relativeOrder: 3,
relativeOrder: 2,
})
this.form.username.bindPrompter(getUsernamePrompter, {
showWhen: state =>
(state.database !== undefined && state.connectionType === ConnectionType.TempCreds) ||
state.connectionType === ConnectionType.DatabaseUser,
relativeOrder: 2,
relativeOrder: 3,
})
this.form.password.bindPrompter(getPasswordPrompter, {
showWhen: state => state.username !== undefined && state.connectionType === ConnectionType.DatabaseUser,
Expand Down Expand Up @@ -121,7 +109,7 @@ export class NotebookConnectionWizard extends Wizard<ConnectionParams> {
}
}
})
this.form.connectionType.bindPrompter(state => getConnectionTypePrompterNB(state?.warehouseType), {
this.form.connectionType.bindPrompter(state => getConnectionTypePrompter(undefined, state?.warehouseType), {
relativeOrder: 3,
})

Expand All @@ -134,12 +122,12 @@ export class NotebookConnectionWizard extends Wizard<ConnectionParams> {
})
this.form.password.bindPrompter(getPasswordPrompter, {
showWhen: state => state.username !== undefined && state.connectionType === ConnectionType.DatabaseUser,
relativeOrder: 5,
relativeOrder: 6,
})

this.form.secret.bindPrompter(state => getSecretPrompter(state.region!.id), {
showWhen: state => state.database !== undefined && state.connectionType === ConnectionType.SecretsManager,
relativeOrder: 6,
relativeOrder: 5,
})
}
}
Expand Down Expand Up @@ -217,16 +205,6 @@ function getConnectionTypePrompter(
}
}

function getConnectionTypePrompterNB(warehouseType?: RedshiftWarehouseType): Prompter<ConnectionType> {
const items: DataQuickPickItem<ConnectionType>[] = Object.values(ConnectionType).map(type => ({
label: type,
data: type,
}))
// Determine which items to use based on warehouseType
const selectedItems = warehouseType === RedshiftWarehouseType.SERVERLESS ? [items[0], items[1]] : items
return createSelectConnectionQuickPick(selectedItems)
}

async function* fetchWarehouses(redshiftClient: DefaultRedshiftClient) {
let serverlessToken: string | undefined
let provisionedToken: string | undefined
Expand Down
40 changes: 10 additions & 30 deletions src/test/redshift/explorer/redshiftWarehouseNode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ describe('redshiftWarehouseNode', function () {
undefined
)
const redshiftNode = new RedshiftNode(redshiftClient)
let connectionWizardStub: RedshiftNodeConnectionWizard
let listDatabasesStub: sinon.SinonStub
let warehouseNode: RedshiftWarehouseNode
let connectionWizardStub: sinon.SinonStub

beforeEach(function () {
listDatabasesStub = sandbox.stub()
Expand All @@ -67,16 +67,11 @@ describe('redshiftWarehouseNode', function () {

afterEach(function () {
sandbox.reset()
connectionWizardStub.restore()
})

it('gets databases for a warehouse and adds a start button', async () => {
connectionWizardStub = { run: () => Promise.resolve(connectionParams) } as RedshiftNodeConnectionWizard
warehouseNode = new RedshiftWarehouseNode(
redshiftNode,
resourceNode,
RedshiftWarehouseType.PROVISIONED,
connectionWizardStub
)
connectionWizardStub = sinon.stub(RedshiftNodeConnectionWizard.prototype, 'run').resolves(connectionParams)
warehouseNode = new RedshiftWarehouseNode(redshiftNode, resourceNode, RedshiftWarehouseType.PROVISIONED)
listDatabasesStub.returns({ promise: () => Promise.resolve(expectedResponse) })

const childNodes = await warehouseNode.getChildren()
Expand All @@ -85,13 +80,8 @@ describe('redshiftWarehouseNode', function () {
})

it('gets databases for a warehouse, adds a start button and a load more button if there are more results', async () => {
connectionWizardStub = { run: () => Promise.resolve(connectionParams) } as RedshiftNodeConnectionWizard
warehouseNode = new RedshiftWarehouseNode(
redshiftNode,
resourceNode,
RedshiftWarehouseType.PROVISIONED,
connectionWizardStub
)
connectionWizardStub = sinon.stub(RedshiftNodeConnectionWizard.prototype, 'run').resolves(connectionParams)
warehouseNode = new RedshiftWarehouseNode(redshiftNode, resourceNode, RedshiftWarehouseType.PROVISIONED)
listDatabasesStub.returns({ promise: () => Promise.resolve(expectedResponseWithNextToken) })

const childNodes = await warehouseNode.getChildren()
Expand All @@ -100,25 +90,15 @@ describe('redshiftWarehouseNode', function () {
})

it('shows a node with retry if user exits wizard', async () => {
connectionWizardStub = { run: () => Promise.resolve(undefined) } as RedshiftNodeConnectionWizard
warehouseNode = new RedshiftWarehouseNode(
redshiftNode,
resourceNode,
RedshiftWarehouseType.PROVISIONED,
connectionWizardStub
)
connectionWizardStub = sinon.stub(RedshiftNodeConnectionWizard.prototype, 'run').resolves(undefined)
warehouseNode = new RedshiftWarehouseNode(redshiftNode, resourceNode, RedshiftWarehouseType.PROVISIONED)
const childNodes = await warehouseNode.getChildren()
verifyRetryNode(childNodes)
})

it('shows a node with retry if there is error fetching databases', async () => {
connectionWizardStub = { run: () => Promise.resolve(connectionParams) } as RedshiftNodeConnectionWizard
warehouseNode = new RedshiftWarehouseNode(
redshiftNode,
resourceNode,
RedshiftWarehouseType.PROVISIONED,
connectionWizardStub
)
connectionWizardStub = sinon.stub(RedshiftNodeConnectionWizard.prototype, 'run').resolves(connectionParams)
warehouseNode = new RedshiftWarehouseNode(redshiftNode, resourceNode, RedshiftWarehouseType.PROVISIONED)
listDatabasesStub.returns({ promise: () => Promise.reject('Failed') })
const childNodes = await warehouseNode.getChildren()
verifyRetryNode(childNodes)
Expand Down