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

feat: create wrapper for SQSClient #9

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ logs
test-results.xml
.env
.idea/
.npmrc
20 changes: 10 additions & 10 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Adobe. All rights reserved.
* Copyright 2023 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
Expand All @@ -13,9 +13,9 @@ import secrets from '@adobe/helix-shared-secrets';
import wrap from '@adobe/helix-shared-wrap';
import { logger } from '@adobe/helix-universal-logger';
import { helixStatus } from '@adobe/helix-status';
import SQSQueue from './sqs-queue.js';
import DB from './db.js'; // Assuming the exported content of './db' is default exported
import PSIClient from './psi-client.js'; // Assuming the exported content of './psi-client' is default exported
import DB from './db.js';
import PSIClient from './psi-client.js';
import queueWrapper from './queue-wrapper.js';

/**
* This is the main function
Expand All @@ -25,14 +25,13 @@ import PSIClient from './psi-client.js'; // Assuming the exported content of './
*/
async function run(request, context) {
const db = DB({
region: process.env.REGION,
region: context.env.REGION,
});
const sqsQueue = SQSQueue();
const { message } = JSON.parse(context.invocation.event.Records[0].body);

const psiClient = PSIClient({
apiKey: process.env.PAGESPEED_API_KEY,
baseUrl: process.env.PAGESPEED_API_BASE_URL,
apiKey: context.env.PAGESPEED_API_KEY,
baseUrl: context.env.PAGESPEED_API_BASE_URL,
});

const site = {
Expand All @@ -44,12 +43,13 @@ async function run(request, context) {
};
const auditResult = await psiClient.runAudit(`https://${site.domain}/${site.path}`);
const auditResultMin = await db.saveAuditIndex(site, auditResult);
await sqsQueue.sendMessage(auditResultMin);
await context.queue.sendAuditResult(auditResultMin);
return new Response('SUCCESS');
}

export const main = wrap(run)
.with(helixStatus)
.with(logger.trace)
.with(logger)
.with(secrets);
.with(secrets)
.with(queueWrapper);
Copy link
Member

Choose a reason for hiding this comment

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

secrets wrapper should be in outer level then queueWrapper otherwise queueWrapper won't have access to the env variables

38 changes: 18 additions & 20 deletions src/util.js → src/queue-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,23 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
const log = (level, message, ...args) => {
const timestamp = new Date().toISOString();

switch (level) {
case 'info':
console.info(`[${timestamp}] INFO: ${message}`, ...args);
break;
case 'error':
console.error(`[${timestamp}] ERROR: ${message}`, ...args);
break;
case 'warn':
console.warn(`[${timestamp}] WARN: ${message}`, ...args);
break;
default:
console.log(`[${timestamp}] ${message}`, ...args);
break;
}
};
'use strict';

export {
log,
};
import SqsQueue from './sqs-queue.js';

export default function queueWrapper(func) {
return async (request, context) => {
const region = context.env.AWS_REGION;
const queueUrl = context.env.AUDIT_RESULTS_QUEUE_URL;
const { log } = context;

if (!queueUrl) {
throw new Error('AUDIT_RESULTS_QUEUE_URL env variable is empty/not provided');
}

context.queue = SqsQueue(region, queueUrl, log);
Copy link
Member

Choose a reason for hiding this comment

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

nitpicking: I would check if context.queue already defined here, and remove the if check in the SQSQueues constructor

Suggested change
context.queue = SqsQueue(region, queueUrl, log);
if (!context.queue) {
context.queue = SqsQueue(region, queueUrl, log);
}

see the suggestion below as well

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@tripodsan thank you! @dzehnder let's stick to the same convention

Copy link
Contributor

Choose a reason for hiding this comment

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

@tripodsan could you give me access to helix-admin-support repo I don t seem to have it

Copy link
Contributor

Choose a reason for hiding this comment

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

We took a look and we will add the queue and storage to the context, but I think we shouldn t add it to the attributes, because I see the storage which is similar to db and queue is added directly to the context. Wdyt @tripodsan ?


return func(request, context);
};
}
36 changes: 12 additions & 24 deletions src/sqs-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,47 +9,35 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import { SQSClient, SendMessageCommand } from '@aws-sdk/client-sqs';
import { SendMessageCommand, SQSClient } from '@aws-sdk/client-sqs';

// Set up the region
const REGION = 'us-east-1'; // change this to your desired region
let sqsClient;

// Create SQS service client object
const sqsClient = new SQSClient({ region: REGION });

// Your SQS queue URL
const queueURL = 'https://sqs.us-east-1.amazonaws.com/282898975672/spacecat-audit-results';
export default function SQSQueue(region, queueUrl, log) {
if (!sqsClient) {
sqsClient = new SQSClient({ region });
log.info(`Creating SQS client in region ${region}`);
}
Copy link
Member

Choose a reason for hiding this comment

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

continues from the suggestion above

Suggested change
export default function SQSQueue(region, queueUrl, log) {
if (!sqsClient) {
sqsClient = new SQSClient({ region });
log.info(`Creating SQS client in region ${region}`);
}
export default function SQSQueue(region, queueUrl, log) {
const sqsClient = new SQSClient({ region });
log.info(`Creating SQS client in region ${region}`);


function SQSQueue() {
async function sendMessage(message) {
async function sendAuditResult(message) {
const body = {
message,
timestamp: new Date().toISOString(),
};

// Set up the parameters for the send message command
const params = {
DelaySeconds: 10,
MessageBody: JSON.stringify(body),
QueueUrl: queueURL,
QueueUrl: queueUrl,
};

try {
const data = await sqsClient.send(new SendMessageCommand(params));
console.log('Success, message sent. MessageID:', data.MessageId);
log.info('Success, message sent. MessageID:', data.MessageId);
} catch (err) {
console.log('Error:', err);
log.error('Error:', err);
throw err;
}

return {
statusCode: 200,
body: JSON.stringify({ message: 'SQS message sent!' }),
};
}
return {
sendMessage,
};
return { sendAuditResult };
}

export default SQSQueue;
Loading