Skip to content
This repository was archived by the owner on May 31, 2024. It is now read-only.

feat: Improve context stack deployment speed#150

Merged
abassyiouni merged 3 commits into
mainfrom
ahmaalba/unnestedContextStack
Nov 17, 2021
Merged

feat: Improve context stack deployment speed#150
abassyiouni merged 3 commits into
mainfrom
ahmaalba/unnestedContextStack

Conversation

@abassyiouni
Copy link
Copy Markdown
Contributor

Description of Changes

Combine nested stacks to single stack to speed up deployment time.

Description of how you validated changes

Deployed cromwell, nextflow contexts and ran workflows to validate changes. Tested log retrieval for workflows as well. Workflows tested:

  • Hello (wdl)
  • Hello (nextflow)
  • rnaseq (nextflow)
  • bam-to-unmapped-bams (wdl)

Context deployment was running on average for 12 minutes rather then 18 during these deployments

Checklist

  • If this change would make any existing documentation invalid, I have included those updates within this PR
  • I have added unit tests that prove my fix is effective or that my feature works
  • I have linted my code before raising the PR
  • Title of this Pull Request follows Conventional Commits standard: https://www.conventionalcommits.org/en/v1.0.0/

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@abassyiouni abassyiouni temporarily deployed to slack November 8, 2021 08:11 Inactive
Comment thread packages/cdk/lib/stacks/engines/batch-stack.ts Outdated
Comment thread packages/cdk/lib/stacks/engines/batch-stack.ts Outdated
/**
* AWS Batch JobQueue to use for running workflows.
*/
readonly jobQueue: IJobQueue;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see this being used anywhere. Is it needed?

Copy link
Copy Markdown
Contributor Author

@abassyiouni abassyiouni Nov 11, 2021

Choose a reason for hiding this comment

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

Yup, its used a few times. Here is one: https://github.com/aws/amazon-genomics-cli/pull/150/files#diff-8623b226bf6f1a3639f3853bdccb79a30a41db59747e010dbe8fc601353f9273R38 . This jobQueue object was previously in the engine options type, which stopped miniwdl's props from extending that type. Removing it and putting it within the props allows for the engine options type to be used across all constructs.

@tneely tneely changed the title feat: Unnested context stack implementation feat: Improve context stack deployment speed Nov 9, 2021
tneely
tneely previously approved these changes Nov 10, 2021
ghawk1ns
ghawk1ns previously approved these changes Nov 15, 2021
@abassyiouni abassyiouni dismissed stale reviews from ghawk1ns and tneely via 6e004d4 November 16, 2021 17:44
@abassyiouni abassyiouni merged commit f7d6c7f into main Nov 17, 2021
@abassyiouni abassyiouni deleted the ahmaalba/unnestedContextStack branch November 17, 2021 03:12
AndreyDovydenko pushed a commit that referenced this pull request Nov 24, 2021
* Unnested context stack implementation

* Class name refactoring
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants