[NEMO-429] SWPP TEAM12 Code Smell Fix #274
Conversation
- runtime/executor/src/main/java/org/apache/nemo/runtime/executor/data/stores/LocalFileStore.java - row 79 - removed throws BlockWriteException
There was a problem hiding this comment.
Hi all, the PR looks good overall, but I've requested some simple changes. Please apply them and check if it works. It also seems like there's a check style issue on the branch. After applying the requested changes, please run mvn clean install -T1C -DskipTests and confirm that it works before submitting your final works. Thanks! 👍
| @@ -264,6 +264,9 @@ public void processElement(final ProcessContext c) throws Exception { | |||
| * Composite transform that wraps the transforms inside the loop. | |||
| * The loop updates the user matrix and the item matrix in each iteration. | |||
| */ | |||
|
|
|||
|
|
|||
|
|
|||
There was a problem hiding this comment.
It seems like these few lines were added by mistake. Could you revert the changes for this file? Thanks! 😄
| @@ -34,7 +34,7 @@ | |||
| import java.util.HashSet; | |||
| import java.util.Map; | |||
| import java.util.Set; | |||
| import java.util.function.IntPredicate; | |||
| import java.util.function.IntPredicate; //NOSONAR | |||
There was a problem hiding this comment.
Let's revert this change on the import line. This isn't applicable. Instead, let's make the terminationCondition variable transient on line 59. You can declare it as private transient IntPredicate... there and it will solve the issue 👍
| private final String path; | ||
| private Path fileName; | ||
| private Path fileName; //NOSONAR | ||
| private List<I> elements; |
There was a problem hiding this comment.
Please revert the change made on line 39, and make the variables declared on line 41 and 42 transient to remove the code smell. You can declare private transient ... to make the changes. Thanks!
|
@tyj9327 I've left some comments |
|
Don't forget to address the comments! @tyj9327 @Imchaemin @YejiAhn @NSYT0607 |
There was a problem hiding this comment.
Others are all fine. Thanks for resolving the comments. One more thing though, the build is failing with a checkstyle error, could you fix this and confirm that the build succeeds before proceeding? I think all will be good to go then. I think it will be solved if you resolve both of my comments. Thanks!
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:check (validate) on project nemo-runtime-executor: Failed during checkstyle execution: There is 1 error reported by Checkstyle 8.16 with checkstyle.xml ruleset.
| @@ -36,10 +36,10 @@ | |||
| * | |||
| * @param <I> input type. | |||
| */ | |||
| public final class HDFSTextFileTransform<I> extends NoWatermarkEmitTransform<I, String> { | |||
| public final class HDFSTextFileTransform<I> extends NoWatermarkEmitTransform<I, String> { //NOSONAR | |||
There was a problem hiding this comment.
Can you remove this NOSONAR comment? 😄
| }, WATERMARK_PERIOD, WATERMARK_PERIOD, TimeUnit.MILLISECONDS); | ||
| this.watermarkTriggerService.scheduleAtFixedRate(() -> | ||
| watermarkTriggered = true | ||
| , WATERMARK_PERIOD, WATERMARK_PERIOD, TimeUnit.MILLISECONDS); |
There was a problem hiding this comment.
I think these lines are causing the checkstyle error. Can you append the comma at the end of line 59, instead of at the beginning of line 60?
JIRA: NEMO-429: TEAM12
Major changes:
Minor changes to note: