-
Notifications
You must be signed in to change notification settings - Fork 28
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
Jenkins/RFS bug fixes for working pipeline #830
Conversation
Signed-off-by: Tanner Lewis <lewijacn@amazon.com>
…ration Signed-off-by: Tanner Lewis <lewijacn@amazon.com>
Signed-off-by: Tanner Lewis <lewijacn@amazon.com>
Signed-off-by: Tanner Lewis <lewijacn@amazon.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #830 +/- ##
=======================================
Coverage 88.18% 88.18%
=======================================
Files 56 56
Lines 3478 3478
=======================================
Hits 3067 3067
Misses 411 411
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Tanner Lewis <lewijacn@amazon.com>
@@ -60,7 +60,7 @@ public static class Args { | |||
public String s3Region; | |||
|
|||
@ParametersDelegate | |||
public ConnectionDetails.TargetArgs targetArgs; | |||
public ConnectionDetails.TargetArgs targetArgs = new ConnectionDetails.TargetArgs(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chelma, is this part of the change you're doing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I just pushed out a PR that does the same thing.
@@ -60,7 +60,7 @@ public static class Args { | |||
public String s3Region; | |||
|
|||
@ParametersDelegate | |||
public ConnectionDetails.TargetArgs targetArgs; | |||
public ConnectionDetails.TargetArgs targetArgs = new ConnectionDetails.TargetArgs(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I just pushed out a PR that does the same thing.
snapshot_result: CommandResult = snapshot.create(wait=True) | ||
assert snapshot_result.success | ||
metadata_result: CommandResult = metadata.migrate() | ||
assert metadata_result.success | ||
backfill_start_result: CommandResult = backfill.start() | ||
assert backfill_start_result.success | ||
backfill_scale_result: CommandResult = backfill.scale(units=10) | ||
assert backfill_scale_result.success |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
I'm curious how you tested this, though. Can you provide some details on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran the Jenkins pipeline on this branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify - @lewijacn confirmed that the test failed at the assertions when the underlying RFS operation failed.
Description
Required fixes to get RFS flow working as well as fix minor Jenkins issues
Issues Resolved
[List any issues this PR will resolve]
Is this a backport? If so, please add backport PR # and/or commits #
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.