Skip to content

chore: adds queue to the worker service pattern#2647

Merged
mergify[bot] merged 18 commits intoaws:mainlinefrom
xar-tol:xar-tol/adds-queue-to-worker
Jul 27, 2021
Merged

chore: adds queue to the worker service pattern#2647
mergify[bot] merged 18 commits intoaws:mainlinefrom
xar-tol:xar-tol/adds-queue-to-worker

Conversation

@xar-tol
Copy link
Copy Markdown
Contributor

@xar-tol xar-tol commented Jul 22, 2021

Adds the option for queues to the base worker structure.

Addresses #2550

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

xar-tol added 4 commits July 22, 2021 09:45
Adds subscribe data structures to template workload.
Adds template function for generating subscription JSON that lists out subscribed topics, their services, and any related queue they may have.
adds validations, excluding init/deploy checks for another pr
adds queue ds
adds queue validators/converters
@xar-tol xar-tol requested a review from a team as a code owner July 22, 2021 16:20
@xar-tol xar-tol requested review from iamhopaul123 and removed request for a team July 22, 2021 16:20
@xar-tol xar-tol changed the title Xar tol/adds queue to worker chore: adds queue to the worker service pattern Jul 22, 2021
Copy link
Copy Markdown
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

This is looking really good. Such complex logic and you've handled it quite well.

I have some comments about how we validate the sns topic subscriptions and unmarshal the durations (retention, delay, timeout), but otherwise this looks good and I can't wait to see the template changes.

Comment thread internal/pkg/deploy/cloudformation/stack/validate.go Outdated
Comment thread internal/pkg/manifest/worker_svc.go
Comment thread internal/pkg/deploy/cloudformation/stack/transformers.go Outdated
Comment thread internal/pkg/deploy/cloudformation/stack/transformers.go Outdated
Comment thread internal/pkg/deploy/cloudformation/stack/transformers_test.go Outdated
Comment thread internal/pkg/deploy/cloudformation/stack/validate.go Outdated
@xar-tol xar-tol requested a review from bvtujo July 22, 2021 22:30
Copy link
Copy Markdown
Contributor

@Lou1415926 Lou1415926 left a comment

Choose a reason for hiding this comment

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

Amazing work! Such complex logic and clear break down of PRs.

I have some thoughts and questions below ⬇️

Comment thread internal/pkg/deploy/cloudformation/stack/validate.go Outdated
Comment thread internal/pkg/manifest/worker_svc.go Outdated
Comment thread internal/pkg/manifest/worker_svc.go Outdated
Comment thread internal/pkg/manifest/worker_svc_test.go
Comment thread internal/pkg/template/workload.go Outdated
Comment thread internal/pkg/template/workload.go Outdated
Comment thread internal/pkg/deploy/cloudformation/stack/validate.go Outdated
Comment thread internal/pkg/deploy/cloudformation/stack/validate.go
Comment thread internal/pkg/deploy/cloudformation/stack/validate_test.go
Comment thread internal/pkg/template/workload.go Outdated
@xar-tol xar-tol requested a review from Lou1415926 July 23, 2021 18:46
Copy link
Copy Markdown
Contributor

@Lou1415926 Lou1415926 left a comment

Choose a reason for hiding this comment

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

Looking great!!! Nothing major - just two questions and a comment on unit test!

Comment thread internal/pkg/deploy/cloudformation/stack/transformers.go Outdated
Comment thread internal/pkg/template/workload.go Outdated
Comment thread internal/pkg/manifest/worker_svc_test.go Outdated
Comment thread internal/pkg/manifest/worker_svc_test.go
xar-tol added 3 commits July 26, 2021 13:38
removes act info from queues due to new json generation
removes names from queues to prevent replacement errors
@bvtujo bvtujo added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jul 26, 2021
Copy link
Copy Markdown
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Looks amazing. Such thorough testing and so well constructed!

Comment thread internal/pkg/deploy/cloudformation/stack/transformers_test.go
@xar-tol xar-tol removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jul 27, 2021
@xar-tol xar-tol added do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. and removed do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. labels Jul 27, 2021
@mergify mergify Bot merged commit 8182f23 into aws:mainline Jul 27, 2021
@xar-tol xar-tol deleted the xar-tol/adds-queue-to-worker branch July 27, 2021 16:32
xar-tol added a commit to xar-tol/copilot-cli that referenced this pull request Aug 5, 2021
Adds the option for queues to the base worker structure.

Addresses aws#2550

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
Adds the option for queues to the base worker structure.

Addresses aws#2550

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants