chore: Split Stateful Single Stack to Multi Stack #216

williamputraintan · 2024-04-11T02:17:34Z

mainly refactoring that shared resource are all in one stack and any other µ-app that require stateful be on different stack (preventing new µ-app feature to step on SharedStack's toe)
wrapping multiple stateful stacks in a class called StatefulStackCollection when defined in stage
refactoring on typescript naming convention

Example on cdk ls command after refactoring:

yarn cdk-stateful ls

OrcaBusStatefulPipeline
OrcaBusStatefulPipeline/BetaDeployment/IcaEventPipeStack (BetaDeployment-IcaEventPipeStack)
OrcaBusStatefulPipeline/BetaDeployment/SharedStack (BetaDeployment-SharedStack)
OrcaBusStatefulPipeline/BetaDeployment/TokenServiceStack (BetaDeployment-TokenServiceStack)
...

resolves #188

williamputraintan · 2024-04-11T02:23:50Z

README.md

+- Name files with camelCase (for example, ebsVolumes.tsx or storage.tsb)
+
+For folder name, we will be using `kebab-case`
+


I have been inconsistent with naming convention, but I think we could follow what AWS suggests.

Folder naming convention is not mentioned, but in general kebab-case is more common. For example, AWS uses kebab-case in their CDK library (import { IVpc } from 'aws-cdk-lib/aws-ec2';)

will refactor on the stateless stack after this PR along with removing the parent (of the µ-app) stack (#157 (comment))

Sound good. I support this.

williamputraintan · 2024-04-11T02:27:43Z

docs/developer/SHARED_RESOURCES.md

@@ -1,44 +1,7 @@
 # Shared Resources



I think we should remove this entirely and have this docs at the folder where the resource is deployed

Feel, 😬 a little bit of double efforts/handling here, yes.

Up to the team with favour the change/remove; ie stick the README closer to the stack/resource that is deployed. At least for these early days taking off phases. We tend to change .. a lot, frequently.

williamputraintan · 2024-04-11T02:31:51Z

This might need a redeployment of resources here as stack id will change.

We could also roll this to all of our account env (dev, stg, prod) once this is approved

victorskl · 2024-04-11T23:54:17Z

.. will review today

victorskl · 2024-04-12T01:05:46Z

reviewing...

victorskl

👍 from me.!

victorskl · 2024-04-12T01:07:28Z

README.md

+- Name files with camelCase (for example, ebsVolumes.tsx or storage.tsb)
+
+For folder name, we will be using `kebab-case`
+


Sound good. I support this.

victorskl · 2024-04-12T01:19:35Z

docs/developer/SHARED_RESOURCES.md

@@ -1,44 +1,7 @@
 # Shared Resources



Feel, 😬 a little bit of double efforts/handling here, yes.

Up to the team with favour the change/remove; ie stick the README closer to the stack/resource that is deployed. At least for these early days taking off phases. We tend to change .. a lot, frequently.

victorskl · 2024-04-12T01:26:15Z

lib/workload/components/vpc/index.ts

We can also remove this VPC lookup function i.e. rather promote to use the vpcProps pass down from the constants.ts and, reconstruct at the stack level. Like the way it does in Token Service stack.

What chu think, Will? Leaving it is fine, too.

Agree on lookup at µ-stack level rather than passing IVpc as a stack props. I feel it is more isolated and it is simpler to get onboard when dev the µ-app rather depending on resource as the props.

I just thought this function is just a quick single-line wrapper of common resource lookup

victorskl · 2024-04-12T01:27:16Z

lib/workload/stateful/stacks/shared/constructs/database/index.ts

@@ -112,7 +112,7 @@ export class Database extends Construct {
  constructor(scope: Construct, id: string, props: DatabaseProps) {
    super(scope, id);

-    const dbSecret = new rds.DatabaseSecret(this, id + 'DbSecret', {


Thank you for cleaning this id thingy up.! 😬

victorskl · 2024-04-12T01:28:20Z

lib/workload/stateful/stacks/shared/constructs/schemaregistry/index.ts

Bye, bye, my dear "component". Hehe. J/K 😬

victorskl · 2024-04-12T01:31:06Z

lib/workload/stateful/stacks/token-service/deploy/stack.ts

@@ -35,7 +36,7 @@ export class TokenServiceStack extends Stack {
    super(scope, id, props);
    this.props = props;

-    this.vpc = Vpc.fromLookup(this, 'MainVpc', props.vpcProps);
+    this.vpc = getVpc(this);


Ah. You want the function one. Please check with the Flo! 😃 Think, he prefer the other way, vpcProps pass down. Leave it with you two.

Hm,... does the function add much?
It's a bit easier for me without... first I don't need to know about any outside function and second it's just a lookup like I'd need to do for any other (provided) resource.
Also, getVpc() hides away configuration details that otherwise would be in the usual, expected spot (config constants), right?

Yup. Since we change to independent "stack" concept, the props pass down become more make sense.

Think, Will agree (somewhere ^ there comment; deprecating this function lookup all together). Perhaps, Will could help make necessary changes and, use vpcProps pass down. I reckon, that should work... But, leaving the final call to Will.

Will remove it! It will be easier for native lookup rather than understanding a custom function.

victorskl · 2024-04-12T01:32:14Z

lib/workload/stateful/statefulStackCollectionClass.ts

+  icaEventPipeStackProps: IcaEventPipeStackProps;
+}
+
+export class StatefulStackCollection {


Oh, the "collection" class. I like this naming.! Good one, Will. 👍

Initially was set to "manager" class, but I think we had enough managers in OrcaBus 🤔

reisingerf

Looks nice and neat! Go for it!

reisingerf · 2024-04-12T02:14:38Z

lib/workload/stateful/stacks/token-service/deploy/stack.ts

@@ -35,7 +36,7 @@ export class TokenServiceStack extends Stack {
    super(scope, id, props);
    this.props = props;

-    this.vpc = Vpc.fromLookup(this, 'MainVpc', props.vpcProps);
+    this.vpc = getVpc(this);


Hm,... does the function add much?
It's a bit easier for me without... first I don't need to know about any outside function and second it's just a lookup like I'd need to do for any other (provided) resource.
Also, getVpc() hides away configuration details that otherwise would be in the usual, expected spot (config constants), right?

williamputraintan · 2024-04-12T07:50:51Z

lib/workload/stateful/statefulStackCollectionClass.ts

+    });
+  }
+
+  private createTemplateProps(env: Environment, serviceName: string): StackProps {


added a few more initial tagging template here for each µ-stack @victorskl @reisingerf

reisingerf · 2024-04-12T07:53:15Z

lib/workload/stateful/statefulStackCollectionClass.ts

+    });
+  }
+
+  private createTemplateProps(env: Environment, serviceName: string): StackProps {


williamputraintan added 6 commits April 10, 2024 17:53

REFACTOR stack

09a894a

test fix

37a5807

tagging for µ-app

eaa5543

Update README.md

036a9e2

folder kebab-case

9275fa1

readme

1729c17

williamputraintan commented Apr 11, 2024

View reviewed changes

williamputraintan added 4 commits April 11, 2024 12:40

solving conflict

e0d118c

Merge branch 'main' into feature/188-refactor-stack

43c57b4

rename back

664d6bd

folder rename

b959de0

williamputraintan requested review from victorskl and reisingerf April 11, 2024 02:46

williamputraintan self-assigned this Apr 11, 2024

williamputraintan added the refactor label Apr 11, 2024

williamputraintan added 2 commits April 11, 2024 16:21

filename

b48ac86

package.json

9bc1b0e

victorskl approved these changes Apr 12, 2024

View reviewed changes

reisingerf approved these changes Apr 12, 2024

View reviewed changes

williamputraintan added 3 commits April 12, 2024 17:06

feedback

4604b75

fromLookup fix

ed1c085

enable lint for ./shared

a7bb6f1

williamputraintan commented Apr 12, 2024

View reviewed changes

reisingerf approved these changes Apr 12, 2024

View reviewed changes

williamputraintan added 2 commits April 12, 2024 18:17

Update statefulStackCollectionClass.ts

7089877

name refinement

4028173

williamputraintan merged commit b1006ba into main Apr 15, 2024
2 checks passed

williamputraintan deleted the feature/188-refactor-stack branch April 15, 2024 01:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Split Stateful Single Stack to Multi Stack #216

chore: Split Stateful Single Stack to Multi Stack #216

williamputraintan commented Apr 11, 2024 •

edited

Loading

williamputraintan Apr 11, 2024 •

edited

Loading

williamputraintan Apr 11, 2024 •

edited

Loading

victorskl Apr 12, 2024

williamputraintan Apr 11, 2024

victorskl Apr 12, 2024

williamputraintan commented Apr 11, 2024

victorskl commented Apr 11, 2024

victorskl commented Apr 12, 2024

victorskl left a comment

victorskl Apr 12, 2024

victorskl Apr 12, 2024

victorskl Apr 12, 2024

williamputraintan Apr 12, 2024

williamputraintan Apr 12, 2024

victorskl Apr 12, 2024

victorskl Apr 12, 2024

victorskl Apr 12, 2024

reisingerf Apr 12, 2024

victorskl Apr 12, 2024 •

edited

Loading

williamputraintan Apr 12, 2024

victorskl Apr 12, 2024

victorskl Apr 12, 2024

williamputraintan Apr 12, 2024

reisingerf left a comment

reisingerf Apr 12, 2024

williamputraintan Apr 12, 2024

reisingerf Apr 12, 2024

reisingerf Apr 12, 2024

		- Name files with camelCase (for example, ebsVolumes.tsx or storage.tsb)

		For folder name, we will be using `kebab-case`

chore: Split Stateful Single Stack to Multi Stack #216

chore: Split Stateful Single Stack to Multi Stack #216

Conversation

williamputraintan commented Apr 11, 2024 • edited Loading

williamputraintan Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

williamputraintan Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamputraintan commented Apr 11, 2024

victorskl commented Apr 11, 2024

victorskl commented Apr 12, 2024

victorskl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victorskl Apr 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reisingerf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamputraintan commented Apr 11, 2024 •

edited

Loading

williamputraintan Apr 11, 2024 •

edited

Loading

williamputraintan Apr 11, 2024 •

edited

Loading

victorskl Apr 12, 2024 •

edited

Loading