-
Notifications
You must be signed in to change notification settings - Fork 129
Sample that demonstrates how to convert CSV files hosted on COS to data stored in a RDBMS #180
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
Conversation
Co-authored-by: Sascha Schwarze <[email protected]>
Co-authored-by: Sascha Schwarze <[email protected]>
Co-authored-by: Sascha Schwarze <[email protected]>
Co-authored-by: Sascha Schwarze <[email protected]>
Co-authored-by: Sascha Schwarze <[email protected]>
Co-authored-by: Sascha Schwarze <[email protected]>
cos-to-sql/app.mjs
Outdated
const pgClient = await getPgClient(secretsManager, smPgSecretId); | ||
|
||
// | ||
// Perform a single SQL insert statement per user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to do bulk inserts to reduce load on the database.
Conceptually, using upserts here might also make sense, but does not need to be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that would a lot of sense for this specific use case. But at the same time I wanted to mimic a flow in which complex information is processed and handled line be line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about your comment. While I kept the loop and individual db operations per line, I adjusted the table and switched to use upserts. Thanks for the feedback
|
||
export async function getPgClient(secretsManager, secretId) { | ||
const fn = "getPgClient "; | ||
const startTime = Date.now(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to after line51.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is your motivation on this @SaschaSchwarze0 ?
Do you want to track independent durations for the SM and the PG init operaion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Yes, I assumed you wanted to measure the secret fetching.
cos-to-sql/utils/db.mjs
Outdated
const startTime = Date.now(); | ||
console.log(`${fn} > firstName: '${firstName}', lastName: '${lastName}'`); | ||
return new Promise(function (resolve, reject) { | ||
const queryText = "INSERT INTO users(firstname,lastname) VALUES($1, $2)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To continue the best practive you have followed elsewhere, what about measuring and logging statement durations ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Sascha Schwarze <[email protected]>
This PR provides