Skip to content
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

chore: remove parent (of µ-app) stack in cdk.Stage and Refactor #221

Merged
merged 5 commits into from
Apr 17, 2024

Conversation

williamputraintan
Copy link
Member

  • Remove intermediary stack between stage and µ-app stack

     yarn cdk-stateless
     
     OrcaBusStatelessPipeline
     OrcaBusStatelessPipeline/BetaDeployment/FileManagerStack (BetaDeployment-FileManagerStack)
     OrcaBusStatelessPipeline/BetaDeployment/MetadataManagerStack (BetaDeployment-MetadataManagerStack)
     OrcaBusStatelessPipeline/BetaDeployment/PostgresManagerStack (BetaDeployment-PostgresManagerStack)
     OrcaBusStatelessPipeline/BetaDeployment/SequenceRunManagerStack (BetaDeployment-SequenceRunManagerStack)
     
  • Lookup on shared resource is move to individual stack

  • Refactoring naming convention for TS

@@ -53,36 +53,57 @@ export class Filemanager extends Stack {
constructor(scope: Construct, id: string, props: FilemanagerProps) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@mmalenic I made tweaks on the Filemanager Props here so that the lookup is now done within the stack. So this stack can independently run without depending on other stack. Perhaps I need a pair of eye if I did this correctly.

Copy link
Member

@victorskl victorskl left a comment

Choose a reason for hiding this comment

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

LGTM for me.
Minor comment to attend.
And 1 joke for laugh, pls ignore.

README.md Outdated Show resolved Hide resolved
lib/workload/stateless/stacks/metadata-manager/README.md Outdated Show resolved Hide resolved
@victorskl victorskl linked an issue Apr 15, 2024 that may be closed by this pull request
Copy link
Member

@mmalenic mmalenic left a comment

Choose a reason for hiding this comment

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

Looks good! Moving lookups to inside the stack makes sense.

Comment on lines 76 to 77
buildEnvironment: {},
// rustLog: props?.rustLog,
Copy link
Member

Choose a reason for hiding this comment

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

You can remove these two properties, currently they aren't being used but I'll add back in if needed.

Comment on lines 105 to 106
buildEnvironment: {},
// rustLog: props?.rustLog,
Copy link
Member

Choose a reason for hiding this comment

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

Same here, i.e. remove.

@williamputraintan williamputraintan merged commit 4ea72cb into main Apr 17, 2024
2 checks passed
@williamputraintan williamputraintan deleted the feature/stateless-stack-refactor branch April 17, 2024 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platform CodePipeline chores
4 participants