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

fix(deadline): adding version check for staging deadline #109

Merged
merged 17 commits into from
Sep 17, 2020

Conversation

yashda
Copy link
Contributor

@yashda yashda commented Aug 30, 2020

Adding minimum supported deadline version check while staging
deadline installers. Now we fail fast if the provided version
is not supported by RFDK.


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

Adding minimum supported deadline version check while staging
deadline installers. Now we fail fast if the provided version
is not supported by RFDK.
@yashda yashda added the contribution/core This is a PR that came from AWS. label Aug 30, 2020
@jusiskin jusiskin requested review from ryyakobe and jusiskin August 31, 2020 15:44
packages/aws-rfdk/lib/core/lib/utils.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/core/lib/utils.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/core/lib/utils.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/lib/stage.ts Outdated Show resolved Hide resolved
ddneilson and others added 4 commits September 1, 2020 18:14
Adding minimum supported deadline version check while staging
deadline installers. Now we fail fast if the provided version
is not supported by RFDK.
@yashda yashda requested a review from ryyakobe September 1, 2020 18:58
Copy link
Contributor

@jusiskin jusiskin left a comment

Choose a reason for hiding this comment

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

This looks great. My main suggestion is around code organization, but there is another concern about input validation.

packages/aws-rfdk/lib/core/lib/utils.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/core/lib/utils.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/core/lib/utils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@ryyakobe ryyakobe 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! Thanks, Yash! Do you think we could change a docstring just a bit more?

packages/aws-rfdk/lib/core/lib/utils.ts Outdated Show resolved Hide resolved
yashda and others added 6 commits September 6, 2020 23:20
- Adds support for using the auto approve GitHub action to automatically
approve PR's from Dependabot
- Adds support for using Dependabot to update the version of any GitHub
actions we are using
Bumps [typescript](https://github.com/Microsoft/TypeScript) from 3.9.7 to 4.0.2.
- [Release notes](https://github.com/Microsoft/TypeScript/releases)
- [Commits](microsoft/TypeScript@v3.9.7...v4.0.2)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore(integ): rewrite end-to-end test to simplify deployment, use fallback values, eliminate output file

* chore(integ): breaks up end-to-end scripts for test components into deploy, test, and teardown stages, use common functions instead of redundant scripts; add fallback to cleanup stacks on test failure

* chore(integ): add summary output of test execution with deployment times

* chore(integ): all tests now assumed to run unless override variable is set

* chore(integ): break end-to-end test into different component scripts

* chore(integ): combinede e2e scripts for test components into one component_e2e.sh script

Co-authored-by: Claire Paine <painec@dev-dsk-painec-2c-9abb0f50.us-west-2.amazon.com>
Co-authored-by: Claire Paine <painec@dev-dsk-painec-2c-041e65bb.us-west-2.amazon.com>
ryyakobe
ryyakobe previously approved these changes Sep 10, 2020
Copy link
Contributor

@ryyakobe ryyakobe left a comment

Choose a reason for hiding this comment

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

Great work! Thanks Yash!

Copy link
Contributor

@jusiskin jusiskin 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 great. I have a few more suggestions. Let me know what you think.

packages/aws-rfdk/lib/deadline/lib/stage.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/lib/version.ts Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/test/version.test.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/test/version.test.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/lib/version.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@jusiskin jusiskin left a comment

Choose a reason for hiding this comment

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

Last few changes - but this is otherwise ready.

packages/aws-rfdk/lib/deadline/lib/version.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/lib/version.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/lib/worker-fleet.ts Outdated Show resolved Hide resolved
jusiskin
jusiskin previously approved these changes Sep 16, 2020
Copy link
Contributor

@jusiskin jusiskin left a comment

Choose a reason for hiding this comment

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

Approved!

Copy link
Contributor

@ryyakobe ryyakobe left a comment

Choose a reason for hiding this comment

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

Great work! Thanks again, Yash!

Copy link
Contributor

@jusiskin jusiskin left a comment

Choose a reason for hiding this comment

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

Approved! Looks great!

@jusiskin jusiskin merged commit 75d0f9f into aws:mainline Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants