-
Notifications
You must be signed in to change notification settings - Fork 1
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
Redeliver release events #498
Conversation
6fdee8b
to
8e85a54
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #498 +/- ##
=============================================
+ Coverage 39.62% 45.40% +5.77%
- Complexity 275 315 +40
=============================================
Files 50 50
Lines 2700 2700
Branches 209 209
=============================================
+ Hits 1070 1226 +156
+ Misses 1544 1381 -163
- Partials 86 93 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -124,29 +124,7 @@ private String getObject(String key) throws IOException { | |||
ResponseInputStream<GetObjectResponse> object = s3Client.getObject(objectRequest); | |||
return IOUtils.toString(object, StandardCharsets.UTF_8); | |||
} | |||
private PushPayload getGitHubPushPayloadByKey(String eventType, String body, String key) throws IOException, NoSuchKeyException { |
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.
Moved this to GitHubDeliveryHelper so I could eliminate some duplication and add a (crude) unit test.
githubdelivery/src/main/java/io/dockstore/githubdelivery/GithubDeliveryHelper.java
Show resolved
Hide resolved
assertEquals("1.0.0", payload.getRelease().getTagName()); | ||
} | ||
|
||
private String readResourceAsString(String resource) throws IOException { |
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.
We might have an equivalent existing method in common.FixtureUtils
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.
We might have an equivalent existing method in common.FixtureUtils
We do, and I tried to use it, but because it's a test class, it's not bundled into commons artifacts that we depend on.
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.
We might have an equivalent existing method in common.FixtureUtils
We do, and I tried to use it, but because it's a test class, it's not bundled into commons artifacts that we depend on.
Doh, thanks for trying!
|
||
private static final Logger LOG = LoggerFactory.getLogger(GithubDeliveryHelper.class); | ||
|
||
private static final ObjectMapper MAPPER = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); |
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.
Seems like a common mapper configuration but I'm not sure how much (if any) dockstore common code we have access to here, so 🤷
githubdelivery/src/main/java/io/dockstore/githubdelivery/GithubDeliveryHelper.java
Outdated
Show resolved
Hide resolved
githubdelivery/src/main/java/io/dockstore/githubdelivery/GithubDeliveryHelper.java
Show resolved
Hide resolved
githubdelivery/src/test/java/io/dockstore/githubdelivery/package-info.java
Show resolved
Hide resolved
Quality Gate failedFailed conditions |
Description
Redeliver release events. Code already delivers push and installation events. Did a little refactoring to avoid code duplication.
Review Instructions
./mvnw install -pl githubdelivery -am
Issue
SEAB-6466
Security
If there are any concerns that require extra attention from the security team, highlight them here.
Please make sure that you've checked the following before submitting your pull request. Thanks!
mvn clean install
in the project that you have modified (until https://ucsc-cgl.atlassian.net/browse/SEAB-5300 adds multi-module support properly)