Switch from Common JS to ES Modules, multiple file exports, use full IPs#3
Switch from Common JS to ES Modules, multiple file exports, use full IPs#3
Conversation
This reverts commit 720e970.
cavis
left a comment
cavis
left a comment
There was a problem hiding this comment.
Couple questions, but looks good to me. And also... huzzah tests!
| return cachedConfigs; | ||
| } catch (error) { | ||
| console.error(`Error loading configurations from S3: ${error.message}`); | ||
| throw error; |
There was a problem hiding this comment.
Okay, so if we fail to load config, just throw an error. And hopefully we retry this batch of S3 log files later.
There was a problem hiding this comment.
yeah, I think that's the right thing - no config, no processing
| }); | ||
|
|
||
| describe("loadConfigs", () => { | ||
| test("should load configurations successfully from S3", async () => { |
| return PODCAST_IDS.includes(parseInt(data["prx-podcast-id"])); | ||
| }); | ||
|
|
||
| const currentFields = [...originalFields]; |
There was a problem hiding this comment.
Github not formatting this diff very nicely... but it seems like you're implying different destinations may get a different set of fields now? (Not the same for everyone).
There was a problem hiding this comment.
Just keeping track of which were the original fields, which were derived from the initial rows in the file, and which are fields we are adding.
I did think at one point that different files could have different rows, but also just looking to making it clearer to future me which cols we added, and when we are adding columns.
There was a problem hiding this comment.
I also moved a few things out of the config loop, and made the initialRows usage clearer, which may make the diff a bit easier to read, and the code slightly more efficient.
| - LogBucket | ||
| - LogPrefix | ||
| - DestinationBucket | ||
| - DestinationPrefix |
There was a problem hiding this comment.
this was unused for permissions
2 participants
Uh oh!
There was an error while loading. Please reload this page.