From cfa0c8b9aff3b99639ac6075a392a9d2bed84308 Mon Sep 17 00:00:00 2001 From: Krishna Acondy Date: Wed, 21 Jul 2021 08:12:34 +0100 Subject: [PATCH] chore(refactor): only fetch job if streaming logs, fix tests, add JSDoc comments --- src/api/viya/pollJobState.ts | 34 +++++++++++++------------- src/api/viya/saveLog.ts | 17 ++++++++----- src/api/viya/spec/pollJobState.spec.ts | 28 +++++++++++++++------ src/api/viya/spec/saveLog.spec.ts | 21 +++------------- src/api/viya/uploadTables.ts | 8 ++++++ 5 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/api/viya/pollJobState.ts b/src/api/viya/pollJobState.ts index b777382..6d66244 100644 --- a/src/api/viya/pollJobState.ts +++ b/src/api/viya/pollJobState.ts @@ -8,7 +8,6 @@ import { saveLog } from './saveLog' import { createWriteStream } from '@sasjs/utils/file' import { WriteStream } from 'fs' import { Link } from '../../types' -import { prefixMessage } from '@sasjs/utils/error' export async function pollJobState( requestClient: RequestClient, @@ -206,25 +205,26 @@ const doPoll = async ( pollCount++ - const jobUrl = postedJob.links.find((l: Link) => l.rel === 'self') - const { result: job } = await requestClient.get( - jobUrl!.href, - authConfig?.access_token - ) + if (pollOptions?.streamLog) { + const jobUrl = postedJob.links.find((l: Link) => l.rel === 'self') + const { result: job } = await requestClient.get( + jobUrl!.href, + authConfig?.access_token + ) - const endLogLine = job.logStatistics?.lineCount ?? 1000000 + const endLogLine = job.logStatistics?.lineCount ?? 1000000 - await saveLog( - postedJob, - requestClient, - pollOptions?.streamLog || false, - startLogLine, - endLogLine, - logStream, - authConfig?.access_token - ) + await saveLog( + postedJob, + requestClient, + startLogLine, + endLogLine, + logStream, + authConfig?.access_token + ) - startLogLine += job.logStatistics.lineCount + startLogLine += endLogLine + } if (debug && printedState !== state) { logger.info('Polling job status...') diff --git a/src/api/viya/saveLog.ts b/src/api/viya/saveLog.ts index 94fbbfd..a659acf 100644 --- a/src/api/viya/saveLog.ts +++ b/src/api/viya/saveLog.ts @@ -4,20 +4,25 @@ import { fetchLog } from '../../utils' import { WriteStream } from 'fs' import { writeStream } from './writeStream' +/** + * Appends logs to a supplied write stream. + * This is useful for getting quick feedback on longer running jobs. + * @param job - the job to fetch logs for + * @param requestClient - the pre-configured HTTP request client + * @param startLine - the line at which to start fetching the log + * @param endLine - the line at which to stop fetching the log + * @param logFileStream - the write stream to which the log is appended + * @accessToken - an optional access token for authentication/authorization + * The access token is not required when fetching logs from the browser. + */ export async function saveLog( job: Job, requestClient: RequestClient, - shouldSaveLog: boolean, startLine: number, endLine: number, logFileStream?: WriteStream, accessToken?: string ) { - console.log('startLine: ', startLine, ' endLine: ', endLine) - if (!shouldSaveLog) { - return - } - if (!accessToken) { throw new Error( `Logs for job ${job.id} cannot be fetched without a valid access token.` diff --git a/src/api/viya/spec/pollJobState.spec.ts b/src/api/viya/spec/pollJobState.spec.ts index dcbd2b9..a415b98 100644 --- a/src/api/viya/spec/pollJobState.spec.ts +++ b/src/api/viya/spec/pollJobState.spec.ts @@ -1,11 +1,12 @@ -import * as fs from 'fs' import { Logger, LogLevel } from '@sasjs/utils' +import * as fileModule from '@sasjs/utils/file' import { RequestClient } from '../../../request/RequestClient' import { mockAuthConfig, mockJob } from './mockResponses' import { pollJobState } from '../pollJobState' import * as getTokensModule from '../../../auth/getTokens' import * as saveLogModule from '../saveLog' import { PollOptions } from '../../../types' +import { WriteStream } from 'fs' const requestClient = new (>RequestClient)() const defaultPollOptions: PollOptions = { @@ -73,7 +74,18 @@ describe('pollJobState', () => { expect(getTokensModule.getTokens).toHaveBeenCalledTimes(3) }) - it('should attempt to fetch and save the log after each poll', async () => { + it('should attempt to fetch and save the log after each poll when streamLog is true', async () => { + mockSimplePoll() + + await pollJobState(requestClient, mockJob, false, mockAuthConfig, { + ...defaultPollOptions, + streamLog: true + }) + + expect(saveLogModule.saveLog).toHaveBeenCalledTimes(2) + }) + + it('should not attempt to fetch and save the log after each poll when streamLog is false', async () => { mockSimplePoll() await pollJobState( @@ -84,7 +96,7 @@ describe('pollJobState', () => { defaultPollOptions ) - expect(saveLogModule.saveLog).toHaveBeenCalledTimes(2) + expect(saveLogModule.saveLog).not.toHaveBeenCalled() }) it('should return the current status when the max poll count is reached', async () => { @@ -133,7 +145,7 @@ describe('pollJobState', () => { defaultPollOptions ) - expect(requestClient.get).toHaveBeenCalledTimes(3) + expect(requestClient.get).toHaveBeenCalledTimes(2) expect(state).toEqual('completed') }) @@ -179,7 +191,7 @@ describe('pollJobState', () => { defaultPollOptions ) - expect(requestClient.get).toHaveBeenCalledTimes(3) + expect(requestClient.get).toHaveBeenCalledTimes(2) expect(state).toEqual('completed') }) @@ -205,7 +217,7 @@ const setupMocks = () => { jest.mock('../../../request/RequestClient') jest.mock('../../../auth/getTokens') jest.mock('../saveLog') - jest.mock('fs') + jest.mock('@sasjs/utils/file') jest .spyOn(requestClient, 'get') @@ -219,8 +231,8 @@ const setupMocks = () => { .spyOn(saveLogModule, 'saveLog') .mockImplementation(() => Promise.resolve()) jest - .spyOn(fs, 'createWriteStream') - .mockImplementation(() => ({} as unknown as fs.WriteStream)) + .spyOn(fileModule, 'createWriteStream') + .mockImplementation(() => Promise.resolve({} as unknown as WriteStream)) } const mockSimplePoll = (runningCount = 2) => { diff --git a/src/api/viya/spec/saveLog.spec.ts b/src/api/viya/spec/saveLog.spec.ts index 4d35c6f..a6c662b 100644 --- a/src/api/viya/spec/saveLog.spec.ts +++ b/src/api/viya/spec/saveLog.spec.ts @@ -15,22 +15,10 @@ describe('saveLog', () => { setupMocks() }) - it('should return immediately if shouldSaveLog is false', async () => { - await saveLog(mockJob, requestClient, false, 0, 100, stream, 't0k3n') - - expect(fetchLogsModule.fetchLog).not.toHaveBeenCalled() - expect(writeStreamModule.writeStream).not.toHaveBeenCalled() - }) - it('should throw an error when a valid access token is not provided', async () => { - const error = await saveLog( - mockJob, - requestClient, - true, - 0, - 100, - stream - ).catch((e) => e) + const error = await saveLog(mockJob, requestClient, 0, 100, stream).catch( + (e) => e + ) expect(error.message).toContain( `Logs for job ${mockJob.id} cannot be fetched without a valid access token.` @@ -41,7 +29,6 @@ describe('saveLog', () => { const error = await saveLog( { ...mockJob, links: mockJob.links.filter((l) => l.rel !== 'log') }, requestClient, - true, 0, 100, stream, @@ -54,7 +41,7 @@ describe('saveLog', () => { }) it('should fetch and save logs to the given path', async () => { - await saveLog(mockJob, requestClient, true, 0, 100, stream, 't0k3n') + await saveLog(mockJob, requestClient, 0, 100, stream, 't0k3n') expect(fetchLogsModule.fetchLog).toHaveBeenCalledWith( requestClient, diff --git a/src/api/viya/uploadTables.ts b/src/api/viya/uploadTables.ts index e7f5b66..b9e4402 100644 --- a/src/api/viya/uploadTables.ts +++ b/src/api/viya/uploadTables.ts @@ -2,6 +2,14 @@ import { prefixMessage } from '@sasjs/utils/error' import { RequestClient } from '../../request/RequestClient' import { convertToCSV } from '../../utils/convertToCsv' +/** + * Uploads tables to SAS as specially formatted CSVs. + * This is more compact than JSON, and easier to read within SAS. + * @param requestClient - the pre-configured HTTP request client + * @param data - the JSON representation of the data to be uploaded + * @param accessToken - an optional access token for authentication/authorization + * The access token is not required when uploading tables from the browser. + */ export async function uploadTables( requestClient: RequestClient, data: any,