-
Notifications
You must be signed in to change notification settings - Fork 0
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
refactor: filemanager deployment code and Makefile #98
Conversation
6a7034c
to
463b10b
Compare
I will wait for rebase, Marko. Then, will review after one go... |
…ager # Conflicts: # lib/workload/stateful/database/component.ts # lib/workload/stateless/filemanager/README.md # lib/workload/stateless/filemanager/database/queries/ingester/aws/update_deleted.sql # lib/workload/stateless/filemanager/database/queries/ingester/aws/update_reordered_for_created.sql # lib/workload/stateless/filemanager/database/queries/ingester/aws/update_reordered_for_deleted.sql # lib/workload/stateless/filemanager/filemanager/src/events/aws/collector_builder.rs # test/stateful/databaseConstruct.test.ts
…groups and pre-compiled sqlx queries
9e675b2
to
360b2b3
Compare
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.
Should be ready for a review now, /cc @victorskl.
@@ -0,0 +1,117 @@ | |||
{ |
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.
FYI, these query-*.json
files are required for filemanager to compile without an active postgres database running, which means that any cdk synth
doesn't need to know about the filemanager's docker compose configuration.
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.
Looking good most of it. Except the db SG refactor may need revert as explain its original intent. Ping me pls if we'd like catchup a bit more focus/details, etc.
], | ||
}, | ||
}); | ||
}); |
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.
Nice! thanks for testing. :)
Thanks for the review @victorskl. I'll revert that SG change and also address the other recommendations. |
Sound good, no worries. |
…manager dependencies
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.
LGTM!
Closes #85.
No rush on this, I can merge after #92.
Changes
cdk_resource_invoke.ts
generic over the type of Lambda function so that it can be used in other contexts and not just Rust functions.