Skip to content

Commit 1d8e3f0

Browse files
yi-jin-0yi-jin-0
authored
refactor(redshift): logging and output messages #3943
Problem: There are similar logging and output message in node explorer. Refactoring code to centralize logging and message output makes it easy for maintenance. Solution: Create message functions and add them in utils.
1 parent 0e7647d commit 1d8e3f0

File tree

5 files changed

+64
-47
lines changed

5 files changed

+64
-47
lines changed

‎src/redshift/activation.ts‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@ import { localize } from '../shared/utilities/vsCodeUtils'
1616
import { RedshiftWarehouseNode } from './explorer/redshiftWarehouseNode'
1717
import { ToolkitError } from '../shared/errors'
1818
import { deleteConnection, updateConnectionParamsState } from './explorer/redshiftState'
19-
import globals from '../shared/extensionGlobals'
2019
import { showViewLogsMessage } from '../shared/utilities/messages'
20+
import { showConnectionMessage } from './messageUtils'
2121

2222
export async function activate(ctx: ExtContext): Promise<void> {
23-
const outputChannel = globals.outputChannel
24-
2523
if ('NotebookEdit' in vscode) {
2624
ctx.extensionContext.subscriptions.push(
2725
vscode.workspace.registerNotebookSerializer('aws-redshift-sql-notebook', new RedshiftNotebookSerializer())
@@ -34,7 +32,7 @@ export async function activate(ctx: ExtContext): Promise<void> {
3432
ctx.extensionContext.subscriptions.push(
3533
Commands.register(
3634
'aws.redshift.notebookConnectClicked',
37-
getNotebookConnectClickedHandler(ctx, redshiftNotebookController, outputChannel)
35+
getNotebookConnectClickedHandler(ctx, redshiftNotebookController)
3836
)
3937
)
4038

@@ -46,7 +44,7 @@ export async function activate(ctx: ExtContext): Promise<void> {
4644
)
4745

4846
ctx.extensionContext.subscriptions.push(
49-
Commands.register('aws.redshift.editConnection', getEditConnectionHandler(outputChannel))
47+
Commands.register('aws.redshift.editConnection', getEditConnectionHandler())
5048
)
5149

5250
ctx.extensionContext.subscriptions.push(
@@ -73,19 +71,14 @@ function getCreateNotebookClickedHandler(redshiftNotebookController: RedshiftNot
7371
}
7472
}
7573

76-
function getNotebookConnectClickedHandler(
77-
ctx: ExtContext,
78-
redshiftNotebookController: RedshiftNotebookController,
79-
outputChannel: vscode.OutputChannel
80-
) {
74+
function getNotebookConnectClickedHandler(ctx: ExtContext, redshiftNotebookController: RedshiftNotebookController) {
8175
return async (cell: vscode.NotebookCell, refreshCellStatusBar: () => void) => {
8276
const warehouseConnectionWizard = new NotebookConnectionWizard(ctx.regionProvider)
8377
let connectionParams: ConnectionParams | undefined = await warehouseConnectionWizard.run()
8478
if (!connectionParams) {
8579
return
8680
}
8781
redshiftNotebookController.redshiftClient = new DefaultRedshiftClient(connectionParams.region!.id)
88-
outputChannel.show(true)
8982
try {
9083
const redshiftClient = (redshiftNotebookController.redshiftClient = new DefaultRedshiftClient(
9184
connectionParams.region!.id
@@ -98,11 +91,9 @@ function getNotebookConnectClickedHandler(
9891
connectionParams.secret = secretArnFetched
9992
}
10093
await redshiftNotebookController.redshiftClient.listDatabases(connectionParams!)
101-
outputChannel.appendLine(`Redshift: connected to: ${connectionParams.warehouseIdentifier}`)
94+
showConnectionMessage(connectionParams.warehouseIdentifier, undefined)
10295
} catch (error) {
103-
outputChannel.appendLine(
104-
`Redshift: failed to connect to: ${connectionParams.warehouseIdentifier} - ${(error as Error).message}`
105-
)
96+
showConnectionMessage(connectionParams.warehouseIdentifier, error as Error)
10697
connectionParams = undefined
10798
}
10899
const edit = new vscode.WorkspaceEdit()
@@ -116,7 +107,7 @@ function getNotebookConnectClickedHandler(
116107
}
117108
}
118109

119-
function getEditConnectionHandler(outputChannel: vscode.OutputChannel) {
110+
function getEditConnectionHandler() {
120111
return async (redshiftWarehouseNode: RedshiftWarehouseNode) => {
121112
try {
122113
const connectionParams = await new RedshiftNodeConnectionWizard(redshiftWarehouseNode).run()
@@ -134,12 +125,7 @@ function getEditConnectionHandler(outputChannel: vscode.OutputChannel) {
134125
await vscode.commands.executeCommand('aws.refreshAwsExplorerNode', redshiftWarehouseNode)
135126
}
136127
} catch (error) {
137-
outputChannel.show(true)
138-
outputChannel.appendLine(
139-
`Redshift: Failed to fetch databases for warehouse ${redshiftWarehouseNode.name} - ${
140-
(error as Error).message
141-
}`
142-
)
128+
showConnectionMessage(redshiftWarehouseNode.name, error as Error)
143129
}
144130
}
145131
}

‎src/redshift/explorer/redshiftDatabaseNode.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,11 @@ import { ConnectionParams } from '../models/models'
1313
import { LoadMoreNode } from '../../shared/treeview/nodes/loadMoreNode'
1414
import { ChildNodeLoader, ChildNodePage } from '../../awsexplorer/childNodeLoader'
1515
import { getIcon } from '../../shared/icons'
16-
import { getLogger } from '../../shared/logger'
1716
import { telemetry } from '../../shared/telemetry/telemetry'
18-
import { showViewLogsMessage } from '../../shared/utilities/messages'
17+
import { showViewLogsFetchMessage } from '../messageUtils'
1918

2019
export class RedshiftDatabaseNode extends AWSTreeNodeBase implements LoadMoreNode {
2120
private readonly childLoader = new ChildNodeLoader(this, token => this.loadPage(token))
22-
private readonly logger = getLogger()
2321

2422
public constructor(
2523
public readonly databaseName: string,
@@ -65,9 +63,7 @@ export class RedshiftDatabaseNode extends AWSTreeNodeBase implements LoadMoreNod
6563
newContinuationToken: listSchemaResponse.NextToken,
6664
}
6765
} catch (error) {
68-
const msg = `Redshift: Failed to fetch schemas for ${this.databaseName}: ${(error as Error).message}`
69-
this.logger.error(msg)
70-
showViewLogsMessage(msg)
66+
showViewLogsFetchMessage('schemas', this.databaseName, error as Error)
7167
return Promise.reject(error)
7268
}
7369
})

‎src/redshift/explorer/redshiftSchemaNode.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@ import { RedshiftTableNode } from './redshiftTableNode'
1212
import { ConnectionParams } from '../models/models'
1313
import { ChildNodeLoader, ChildNodePage } from '../../awsexplorer/childNodeLoader'
1414
import { LoadMoreNode } from '../../shared/treeview/nodes/loadMoreNode'
15-
import { getLogger } from '../../shared/logger'
1615
import { telemetry } from '../../shared/telemetry/telemetry'
1716
import { getIcon } from '../../shared/icons'
18-
import { showViewLogsMessage } from '../../shared/utilities/messages'
17+
import { showViewLogsFetchMessage } from '../messageUtils'
1918

2019
export class RedshiftSchemaNode extends AWSTreeNodeBase implements LoadMoreNode {
2120
private readonly childLoader = new ChildNodeLoader(this, token => this.loadPage(token))
22-
private readonly logger = getLogger()
2321
public constructor(
2422
public readonly schemaName: string,
2523
public readonly redshiftClient: DefaultRedshiftClient,
@@ -64,9 +62,7 @@ export class RedshiftSchemaNode extends AWSTreeNodeBase implements LoadMoreNode
6462
newContinuationToken: listTablesResponse.NextToken,
6563
}
6664
} catch (error) {
67-
const msg = `Redshift: Failed to fetch tables for ${this.schemaName}: ${(error as Error).message}`
68-
this.logger.error(msg)
69-
showViewLogsMessage(msg)
65+
showViewLogsFetchMessage('tables', this.schemaName, error as Error)
7066
return Promise.reject(error)
7167
}
7268
})

‎src/redshift/explorer/redshiftWarehouseNode.ts‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@ import { RedshiftNodeConnectionWizard } from '../wizards/connectionWizard'
1717
import { ListDatabasesResponse } from 'aws-sdk/clients/redshiftdata'
1818
import { getIcon } from '../../shared/icons'
1919
import { AWSCommandTreeNode } from '../../shared/treeview/nodes/awsCommandTreeNode'
20-
import { getLogger } from '../../shared/logger'
2120
import { telemetry } from '../../shared/telemetry/telemetry'
2221
import { deleteConnection, getConnectionParamsState, updateConnectionParamsState } from './redshiftState'
23-
import globals from '../../shared/extensionGlobals'
22+
import { createLogsConnectionMessage, showConnectionMessage } from '../messageUtils'
2423

2524
export class CreateNotebookNode extends AWSCommandTreeNode {
2625
constructor(parent: RedshiftWarehouseNode) {
@@ -99,11 +98,7 @@ export class RedshiftWarehouseNode extends AWSTreeNodeBase implements AWSResourc
9998
newChildren: childNodes,
10099
}
101100
} catch (error) {
102-
getLogger().error(
103-
`Redshift: Failed to fetch databases for warehouse ${this.redshiftWarehouse.name} - ${
104-
(error as Error).message
105-
}`
106-
)
101+
createLogsConnectionMessage(this.redshiftWarehouse.name, error as Error)
107102
return Promise.reject(error)
108103
}
109104
})
@@ -145,12 +140,7 @@ export class RedshiftWarehouseNode extends AWSTreeNodeBase implements AWSResourc
145140
await updateConnectionParamsState(this.arn, this.connectionParams)
146141
return childNodes
147142
} catch (error) {
148-
globals.outputChannel.show(true)
149-
const msg = `Redshift: Failed to fetch databases for warehouse ${this.redshiftWarehouse.name} - ${
150-
(error as Error).message
151-
}`
152-
getLogger().error(msg)
153-
globals.outputChannel.appendLine(msg)
143+
showConnectionMessage(this.redshiftWarehouse.name, error as Error)
154144
await updateConnectionParamsState(this.arn, undefined)
155145
return this.getRetryNode()
156146
}

‎src/redshift/messageUtils.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import globals from '../shared/extensionGlobals'
7+
import { getLogger } from '../shared/logger'
8+
import { showViewLogsMessage } from '../shared/utilities/messages'
9+
10+
/**
11+
* Show the connection message in output channel. Log the error message if connection fails.
12+
* @param warehouseIdentifier
13+
* @param error
14+
*/
15+
export function showConnectionMessage(warehouseIdentifier: string, error: Error | undefined) {
16+
const outputChannel = globals.outputChannel
17+
outputChannel.show(true)
18+
const outputMessage = createLogsConnectionMessage(warehouseIdentifier, error)
19+
outputChannel.appendLine(outputMessage)
20+
}
21+
22+
/**
23+
* Create the connection message and log the error if the connection fails.
24+
* @param warehouseIdentifier
25+
* @param error
26+
* @returns the connection message to show
27+
*/
28+
export function createLogsConnectionMessage(warehouseIdentifier: string, error: Error | undefined): string {
29+
let connectionMessage = ''
30+
if (!error) {
31+
connectionMessage = `Redshift: connected to: ${warehouseIdentifier}`
32+
} else {
33+
connectionMessage = `Redshift: failed to connect to: ${warehouseIdentifier}: ${(error as Error).message}`
34+
getLogger().error(connectionMessage)
35+
}
36+
return connectionMessage
37+
}
38+
39+
/**
40+
* Log the fetching error message and show a window with "View Logs" button
41+
* @param fetchType
42+
* @param identifier
43+
* @param error
44+
*/
45+
export function showViewLogsFetchMessage(fetchType: string, identifier: string, error: Error) {
46+
const message = `Redshift: failed to fetch ${fetchType} for ${identifier}: ${(error as Error).message}`
47+
getLogger().error(message)
48+
showViewLogsMessage(message)
49+
}

0 commit comments

Comments
 (0)