You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2021/04/06 16:34:38 UTC

[GitHub] [submarine] aeioulisa opened a new pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

aeioulisa opened a new pull request #552:
URL: https://github.com/apache/submarine/pull/552


   ### What is this PR for?
   Update some dependence cause vulnerability
   
   
   ### What type of PR is it?
   [Improvement]
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-696
   
   ### How should this be tested?
   https://travis-ci.org/github/aeioulisa/submarine/builds/766093669
   
   ### Screenshots (if appropriate)
   
   ### Questions:
   * Does the licenses files need update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] cxorm commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
cxorm commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r612098316



##########
File path: submarine-client/src/main/java/org/apache/submarine/client/cli/param/runjob/RunJobParameters.java
##########
@@ -347,7 +347,7 @@ public boolean isDistributed() {
   @VisibleForTesting
   public static class UnderscoreConverterPropertyUtils extends PropertyUtils {
     @Override
-    public Property getProperty(Class<? extends Object> type, String name) throws IntrospectionException {

Review comment:
       > We need to be careful about binary compatibility for public APIs, for they could throw NoSuchMethodException during runtime, or cause compilation error during compile time.
   
   Thanks @jojochuang for the catch, I agree with it.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
xunliu commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r612077586



##########
File path: submarine-server/server-submitter/submitter-yarn/pom.xml
##########
@@ -27,7 +27,6 @@
   <artifactId>submarine-submitter-yarn</artifactId>
   <version>0.6.0-SNAPSHOT</version>

Review comment:
       In addition, The community has decide to support k8s more time.
   so there is not need speed time on yarn in the feature.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] aeioulisa commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
aeioulisa commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r608829079



##########
File path: submarine-client/src/main/java/org/apache/submarine/client/cli/param/runjob/RunJobParameters.java
##########
@@ -347,7 +347,7 @@ public boolean isDistributed() {
   @VisibleForTesting
   public static class UnderscoreConverterPropertyUtils extends PropertyUtils {
     @Override
-    public Property getProperty(Class<? extends Object> type, String name) throws IntrospectionException {

Review comment:
       Snakeyaml doesn't throw exception on getProperty since [version 1.19](https://www.javadoc.io/doc/org.yaml/snakeyaml/1.19/org/yaml/snakeyaml/constructor/Constructor.ConstructMapping.html)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] jojochuang commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r612093449



##########
File path: submarine-client/src/main/java/org/apache/submarine/client/cli/param/runjob/RunJobParameters.java
##########
@@ -347,7 +347,7 @@ public boolean isDistributed() {
   @VisibleForTesting
   public static class UnderscoreConverterPropertyUtils extends PropertyUtils {
     @Override
-    public Property getProperty(Class<? extends Object> type, String name) throws IntrospectionException {

Review comment:
       We need to be careful about binary compatibility for public APIs, for they could throw NoSuchMethodException during runtime, or cause compilation error during compile time.
   
   However, this particular case is fine:
   https://stackoverflow.com/questions/14734345/changing-the-interface-without-recompiling-the-implementing-class
   `Changes to the throws clause of methods or constructors do not break compatibility with pre-existing binaries; these clauses are checked only at compile time.`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] cxorm commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
cxorm commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r609229911



##########
File path: submarine-client/src/main/java/org/apache/submarine/client/cli/param/runjob/RunJobParameters.java
##########
@@ -347,7 +347,7 @@ public boolean isDistributed() {
   @VisibleForTesting
   public static class UnderscoreConverterPropertyUtils extends PropertyUtils {
     @Override
-    public Property getProperty(Class<? extends Object> type, String name) throws IntrospectionException {

Review comment:
       Thank you @aeioulisa for providing it, no block in my side.
   I'm +1 for this PR.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] cxorm commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
cxorm commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r612098316



##########
File path: submarine-client/src/main/java/org/apache/submarine/client/cli/param/runjob/RunJobParameters.java
##########
@@ -347,7 +347,7 @@ public boolean isDistributed() {
   @VisibleForTesting
   public static class UnderscoreConverterPropertyUtils extends PropertyUtils {
     @Override
-    public Property getProperty(Class<? extends Object> type, String name) throws IntrospectionException {

Review comment:
       > We need to be careful about binary compatibility for public APIs, for they could throw NoSuchMethodException during runtime, or cause compilation error during compile time.
   
   Thanks @jojochuang for the catch, I agree to it.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] cxorm commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
cxorm commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r608287996



##########
File path: submarine-client/src/main/java/org/apache/submarine/client/cli/param/runjob/RunJobParameters.java
##########
@@ -347,7 +347,7 @@ public boolean isDistributed() {
   @VisibleForTesting
   public static class UnderscoreConverterPropertyUtils extends PropertyUtils {
     @Override
-    public Property getProperty(Class<? extends Object> type, String name) throws IntrospectionException {

Review comment:
       Could you be so kind as to let me know the reason of removing this Exception ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] asfgit closed pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #552:
URL: https://github.com/apache/submarine/pull/552


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] jojochuang commented on a change in pull request #552: SUBMARINE-696. Vulnerability upgrade recommended

Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #552:
URL: https://github.com/apache/submarine/pull/552#discussion_r612091309



##########
File path: pom.xml
##########
@@ -69,7 +69,7 @@
     <tony.version>0.3.25</tony.version>
     <jersey.version>2.27</jersey.version>
     <jersey.servlet.version>1.13</jersey.servlet.version>
-    <jetty.version>9.4.18.v20190429</jetty.version>
+    <jetty.version>9.4.35.v20201120</jetty.version>

Review comment:
       Let's merge it. But there's new Jetty CVE affecting versions below 9.4.39. We can raise another jira to update again.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org