Skip to content

TDI-17/TDI-34 File parse #12

Merged
chelsea-EYDS merged 2 commits intomainfrom
tdiparse
Nov 25, 2022
Merged

TDI-17/TDI-34 File parse #12
chelsea-EYDS merged 2 commits intomainfrom
tdiparse

Conversation

@chelsea-EYDS
Copy link
Contributor

@chelsea-EYDS chelsea-EYDS commented Nov 19, 2022

What has changed:

Added class definitions for TDI17/TDI34 files.
Added sample files to a root directory (could not access sharepoint files from the terminal)
Added Makefile commands to parse local. Parsing requires a file path and file type, either passed in the event, or, set as env vars. Env vars are set for local dev in makefile cmd.

@chelsea-EYDS chelsea-EYDS changed the title TDI-17 File parse TDI-17/TDI-34 File parse Nov 22, 2022
@chelsea-EYDS chelsea-EYDS marked this pull request as draft November 22, 2022 20:13
@chelsea-EYDS chelsea-EYDS force-pushed the tdiparse branch 6 times, most recently from ac3d1dc to f8e7b01 Compare November 22, 2022 20:45
@chelsea-EYDS chelsea-EYDS marked this pull request as ready for review November 22, 2022 20:45
@chelsea-EYDS chelsea-EYDS force-pushed the tdiparse branch 2 times, most recently from 93384f1 to e778ec8 Compare November 22, 2022 21:22
Comment on lines 6 to 15
import { TDI17Record } from '../tdi-parser/tdi17/TDI17Record';
import { TDI17Header } from '../tdi-parser/tdi17/TDI17Header';
import { TDI17Details } from '../tdi-parser/tdi17/TDI17Details';
import { TDI17Trailer } from '../tdi-parser/tdi17/TDI17Trailer';
import { TDI34Record } from '../tdi-parser/tdi34/TDI34Record';
import { TDI34Header } from '../tdi-parser/tdi34/TDI34Header';
import { TDI34Details } from '../tdi-parser/tdi34/TDI34Details';
import { TDI34Trailer } from '../tdi-parser/tdi34/TDI34Trailer';
import { TDI34Enum } from './const/tdi34.const';
import { TDI17Enum } from './const/tdi17.const';
Copy link
Contributor

Choose a reason for hiding this comment

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

could be imported as a single module for 17 and 34 each.

appLogger.log({ event });
appLogger.log({ context });

const filetype = event?.type ?? process.env.TDI_TYPE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion - This line should throw an error if the event type does not exist. Defaulting should not be an option

const outputFile = Buffer.from(JSON.stringify(output));

appLogger.log(outputFile);
await s3manager.putObject(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer this to have it's own try catch.

Comment on lines 61 to 62
const header = TDI_17 ? new TDI17Header({}) : new TDI34Header({});
const trailer = TDI_17 ? new TDI17Trailer({}) : new TDI34Trailer({});
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to store the header and trailer for TDI files.

The transactions line items is all we want.

Comment on lines 86 to 54
const record = TDI_17 ? new TDI17Record({}) : new TDI34Record({});

record.details = TDI_17 ? tdi17RecordDetails : tdi34RecordDetails;
record.trailer = trailer;
record.header = header;

return {
header: record.header.resource,
details: record.details.map((itm: { resource: unknown }) => itm.resource),
trailer: record.trailer.resource,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

we need only the array of details.

@@ -0,0 +1,200 @@
import {
Copy link
Contributor

Choose a reason for hiding this comment

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

create folder level index files and export from them.

@@ -0,0 +1,121 @@
import {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could skip the header and trailer as we don't have any use for it YET. So we can leave it here and use if we need it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay sounds good. I thought we might want the file header as there is the file creation date. So for now I will add an index, and export only the details for 17/34?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

The date seems to also be in each of the transaction items, so it might just be redundant.
Is the header date different or providing us any additional context? 🤔

# Add Local Tdi Files
# ===================================

put-local-tdi17:
Copy link
Contributor

Choose a reason for hiding this comment

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

the make command can also accept paths

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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


await uploadParsedTDI(TDI_FILE, s3manager, output, appLogger);
} catch (e) {
appLogger.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

we should exit the handler here gracefully.

Copy link
Contributor

Choose a reason for hiding this comment

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

or pass the error back to the root fn and exit there

Copy link
Contributor

Choose a reason for hiding this comment

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

It is also okay to not handle it because we will see the error in the console, but more logs and structure will help debugging in the future

return {
TYPE: fileType,
BUCKET: 'bc-pcc-data-files-local',
LOCALSTACK_PATH: `${fileType.toLowerCase()}/${fileType.toUpperCase()}.TXT`,
Copy link
Contributor

Choose a reason for hiding this comment

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

to be removed all together for providing flexible testing

export const generateEnum =(fileType: string)=> {
return {
TYPE: fileType,
BUCKET: 'bc-pcc-data-files-local',
Copy link
Contributor

Choose a reason for hiding this comment

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

Bucket name needs to change by ENV

Comment on lines 22 to 23
"parseTDI17": "NODE_ENV=local RUNTIME_ENV=local LAMBDA_ARG=TDI17 ts-node ./scripts/handler.ts",
"parseTDI34": "NODE_ENV=local RUNTIME_ENV=local LAMBDA_ARG=TDI34 ts-node ./scripts/handler.ts"
Copy link
Contributor

Choose a reason for hiding this comment

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

ts-node ./scripts/handler.ts console.log(handler.({type: "TDI34", path: "pcc-bc-xxxx-local"}))

something like this

@@ -0,0 +1,7 @@
import {handler} from '../src/lambdas/parseTDI'
Copy link
Contributor

Choose a reason for hiding this comment

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

Proabbaly dont need this file

Makefile Outdated
# ===================================

parse-local-tdi17:
cd './apps/backend' && yarn run parseTDI17
Copy link
Contributor

Choose a reason for hiding this comment

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

node -e 'require("./lambdas/parseTDI").handler({type: "TDI17", path: "/xasxasx/xasxas/xasxas"})'"

@chelsea-EYDS chelsea-EYDS merged commit b14f944 into main Nov 25, 2022
@chelsea-EYDS chelsea-EYDS deleted the tdiparse branch January 10, 2023 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants