Mantis runtime compile warn fix #468

g1rjeevan · 2023-06-29T03:03:06Z

Context

Mantis Runtime Module update on the removing of the warnings fix. Refer issue #396

Below update includes Updating of the deprecated code to latest, changing to method reference from lambda, removing used import, generifying objects and updating javadoc of the methods.

Checklist

./gradlew build compiles code correctly
Added new tests where applicable
./gradlew test passes all tests
Extended README or added javadocs where applicable

sundargates · 2023-06-29T04:36:47Z

mantis-runtime/src/main/java/io/mantisrx/runtime/command/CreateZipFile.java

-                is.close();
+                Objects.requireNonNull(is).close();


if (is != null) is.close()

Update the code as suggested.

sundargates · 2023-06-29T04:37:37Z

mantis-runtime/src/main/java/io/mantisrx/runtime/command/LoadValidateCreateDir.java

@@ -96,7 +88,7 @@ public void execute() throws CommandException {
                    File jobDescriptor = new File(fileLoop.getParent() + "/" + jsonFile);
                    new CreateJobDescriptorFile(job, jobDescriptor, fileVersion, fileBase).execute();
                } catch (Exception e) {
-                    System.out.println("Got an error " + e.toString());
+                    System.out.println("Got an error " + e.getMessage());


Get rid of System.out.println. Use log.error instead.

Should I use Logger instead of System.out.println inside mantis runtime module in all places used?

Yes, let's try to eliminate System::out usages in the runtime module. Let's do that in a separate PR though. This PR is big enough already.

Added the logger.

sundargates · 2023-06-29T04:39:06Z

mantis-runtime/src/main/java/io/mantisrx/runtime/sink/ServerSentEventsSink.java

-        if (runNewSseServerImpl(context.getWorkerInfo().getJobName())) {
+        if (runNewSseServerImpl(context.getWorkerInfo().getJobClusterName())) {


nit: What's the rationale for this change?

getJobName() is deprecated and links to use getJobClusterName() , But both getJobName refers the same variable which is jobName string value.

sundargates · 2023-06-29T04:40:11Z

mantis-runtime/src/main/java/io/mantisrx/runtime/sink/ServerSentEventRequestHandler.java

@@ -213,6 +214,7 @@ public Observable<Void> handle(HttpServerRequest<ByteBuf> request,
        if (queryParameters != null && queryParameters.containsKey(ENABLE_PINGS_PARAM)) {
            // enablePings
            String enablePings = queryParameters.get(ENABLE_PINGS_PARAM).get(0);
+            //Code logic can be improved here.


Add a todo comment.

Updated, Please verify and resolve.

github-actions · 2023-06-29T04:42:51Z

Test Results

126 files ±0 126 suites ±0 6m 30s ⏱️ +36s
536 tests ±0 526 ✔️ - 1 8 💤 ±0 2 ❌ +1
538 runs +1 528 ✔️ ±0 8 💤 ±0 2 ❌ +1

For more details on these failures, see this check.

Results for commit 6925a5d. ± Comparison against base commit 91791b2.

♻️ This comment has been updated with latest results.

sundargates · 2023-06-29T04:43:05Z

mantis-runtime/src/main/java/io/mantisrx/runtime/core/MantisStreamImpl.java

+            Set<OperandNode<?>> successorsNodes = graphDag.successors(n);
+            if (successorsNodes.size() == 0) {
                continue;


what's the need for changing this variable name?

@liuml07 suggested preferring readability over more concise code in the issue.

sundargates

LGTM! Please address the comments below. I'll take one final pass after you address the comments.

Thank you for your contribution.

… into mantis-runtime-compile-warn-fix # Conflicts: # mantis-runtime/src/main/java/io/mantisrx/runtime/command/CreateZipFile.java # mantis-runtime/src/main/java/io/mantisrx/runtime/command/LoadValidateCreateDir.java # mantis-runtime/src/main/java/io/mantisrx/runtime/sink/ServerSentEventRequestHandler.java

g1rjeevan · 2023-06-29T08:21:53Z

LGTM! Please address the comments below. I'll take one final pass after you address the comments.

Thank you for your contribution.

Thank you.

sundargates

Please address the last set of comments and feel free to push it.

sundargates · 2023-06-29T13:19:31Z

mantis-runtime/src/main/java/io/mantisrx/runtime/command/CreateZipFile.java

-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
+import java.io.*;


Let's not use star imports.

sundargates · 2023-06-29T13:20:57Z

mantis-runtime/src/main/java/io/mantisrx/runtime/sink/ServerSentEventRequestHandler.java

@@ -129,7 +129,7 @@ public Observable<Void> handle(HttpServerRequest<ByteBuf> request,

        // decouple the observable on a separate thread and add backpressure handling
        String decoupleSSE = "false";//ServiceRegistry.INSTANCE.getPropertiesService().getStringValue("sse.decouple", "false");
-        //Note: Below condition would be always false during if condition.
+        //Todo Note: Below condition would be always false during if condition.


I think this is the syntax for a TODO comment. https://www.jetbrains.com/help/idea/using-todo.html

g1rjeevan requested review from nickmahilani, jeffchao, piygoyal, calvin681, sundargates, Andyz26, hmitnflx, markcho, liuml07 and fdc-ntflx as code owners June 29, 2023 03:03

g1rjeevan had a problem deploying to Integrate Pull Request June 29, 2023 03:03 — with GitHub Actions Failure

Fix compile warnings on mantis runtime module

58853a9

g1rjeevan force-pushed the mantis-runtime-compile-warn-fix branch from db11f3c to 58853a9 Compare June 29, 2023 03:07

g1rjeevan had a problem deploying to Integrate Pull Request June 29, 2023 03:07 — with GitHub Actions Failure

sundargates reviewed Jun 29, 2023

View reviewed changes

g1rjeevan added 2 commits June 29, 2023 13:42

Fix compile warnings on mantis runtime module

ea8eb8b

g1rjeevan had a problem deploying to Integrate Pull Request June 29, 2023 08:15 — with GitHub Actions Failure

sundargates approved these changes Jun 29, 2023

View reviewed changes

Fix compile warnings on mantis runtime module

c17abe6

g1rjeevan had a problem deploying to Integrate Pull Request June 29, 2023 16:30 — with GitHub Actions Failure

Merge branch 'Netflix:master' into mantis-runtime-compile-warn-fix

10cc671

g1rjeevan had a problem deploying to Integrate Pull Request July 2, 2023 15:01 — with GitHub Actions Failure

sundargates merged commit 8053059 into Netflix:master Jul 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mantis runtime compile warn fix #468

Mantis runtime compile warn fix #468

g1rjeevan commented Jun 29, 2023 •

edited

Loading

sundargates Jun 29, 2023

g1rjeevan Jun 29, 2023

sundargates Jun 29, 2023 •

edited

Loading

g1rjeevan Jun 29, 2023

g1rjeevan Jun 29, 2023

sundargates Jun 29, 2023

g1rjeevan Jun 29, 2023

sundargates Jun 29, 2023 •

edited

Loading

g1rjeevan Jun 29, 2023

sundargates Jun 29, 2023

g1rjeevan Jun 29, 2023 •

edited

Loading

github-actions bot commented Jun 29, 2023 •

edited

Loading

sundargates Jun 29, 2023

g1rjeevan Jun 29, 2023

sundargates left a comment

g1rjeevan commented Jun 29, 2023

sundargates left a comment

sundargates Jun 29, 2023

g1rjeevan Jul 2, 2023

sundargates Jun 29, 2023

g1rjeevan Jul 2, 2023

		if (runNewSseServerImpl(context.getWorkerInfo().getJobName())) {
		if (runNewSseServerImpl(context.getWorkerInfo().getJobClusterName())) {

Mantis runtime compile warn fix #468

Mantis runtime compile warn fix #468

Conversation

g1rjeevan commented Jun 29, 2023 • edited Loading

Context

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sundargates Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sundargates Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g1rjeevan Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jun 29, 2023 • edited Loading

Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sundargates left a comment

Choose a reason for hiding this comment

g1rjeevan commented Jun 29, 2023

sundargates left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g1rjeevan commented Jun 29, 2023 •

edited

Loading

sundargates Jun 29, 2023 •

edited

Loading

sundargates Jun 29, 2023 •

edited

Loading

g1rjeevan Jun 29, 2023 •

edited

Loading

github-actions bot commented Jun 29, 2023 •

edited

Loading