You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2022/11/30 04:53:05 UTC

[GitHub] [zeppelin] jongyoul opened a new pull request, #4520: [MINOR] Fix Appveyor error

jongyoul opened a new pull request, #4520:
URL: https://github.com/apache/zeppelin/pull/4520

   ### What is this PR for?
   Fix the issue for appveyor which has an error result.
   
   ### What type of PR is it?
   Bug Fix
   
   ### Todos
   * [x] - Fix .appveyor.yml
   
   ### What is the Jira issue?
   N/A
   
   ### How should this be tested?
   Windows CI passes
   
   ### Questions:
   * Does the licenses files need to 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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
Reamer commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1331765400

   Nice Maven option that I didn't know about until now.
   It would be very elegant if we also changed `-Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn` to `--no-transfer-progress` in the GitHub CI.
   
   https://github.com/apache/zeppelin/blob/364c8554a66d83446025235c2f7d0e3b223db121/.github/workflows/core.yml#L14
   
   I'll prepare a 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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1344262566

   @Reamer BTW, I have checked and found some workarounds, fortunately. Let me make a PR soon. 


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1039334429


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>

Review Comment:
   Oh! I honestly merged this setting after adding several places at first. But I also think it's good. I'll add similar settings in the proper places then.



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1036799642


##########
.appveyor.yml:
##########
@@ -31,4 +31,4 @@ install:
 
 build_script:
   - echo "Build"
-  - ps: ./mvnw clean package -DskipTests
+  - ps: ./mvnw clean package -DskipTests -B

Review Comment:
   ```suggestion
     - ps: ./mvnw clean package -DskipTests -B -q
   ```



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1331767226

   Yes, sometimes quite annoying when we see the logs :-)


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul closed pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul closed pull request #4520: [MINOR] Fix Appveyor error
URL: https://github.com/apache/zeppelin/pull/4520


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] zjffdu commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
zjffdu commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1039360627


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>

Review Comment:
   I think it is possible to put it in root pom's `pluginManagement` as default configuration, and sub module can customize its configuration. 



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1039346702


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>

Review Comment:
   I also don't think so. we should override parent configuration sometimes so I think it could be fine to have separate configuration for every sub-project.



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] pan3793 commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1038899151


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>
+            <recompileMode>all</recompileMode>

Review Comment:
   is it necessary? incremental mode should be faster



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1345098798

   @Reamer @zjffdu I think it archives its own purpose to make appveyor healthy 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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1336178535

   @Reamer @zjffdu This PR makes appveyor back to normal. It was a problem that scala compiler prints warning messages directly and it's judged as error in appveyor. I disabled to print warnings as it's general warning messages. Could you please check kindly check 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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
Reamer commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1039345155


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>

Review Comment:
   If we create a uniform configuration across all sub-modules, then I would definitely prefer a configuration in the parent pom. But I don't know if this is possible.



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1344244250

   @Reamer BTW, in the case of the core module, does the PR fix them as well?


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
Reamer commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1344258895

   > @Reamer BTW, in the case of the core module, does the PR fix them as well?
   
   unfortunately not


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1038971691


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>
+            <recompileMode>all</recompileMode>

Review Comment:
   Agreed with the performance. But it causes failure as it shows an error when zinc is not downloaded for the first time. So I checked it. Moreover, we are developing scala code but it's not a heavy job so I think it should be fine. Otherwise, we can introduce some CI profile and enable it on it but I don't know if it's really necessary



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
Reamer commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1039331979


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>

Review Comment:
   Thank you for solving the CI problem.
   
   I would do the configuration of this plugin in the sub-projects. In most Scala subprojects there is already a custom compiler configuration and the settings from the parent pom.xml should have no effect there. It can be confusing if there is compiler configuration in two places.



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul commented on code in PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#discussion_r1043970325


##########
pom.xml:
##########
@@ -1585,6 +1585,14 @@
           <groupId>net.alchim31.maven</groupId>
           <artifactId>scala-maven-plugin</artifactId>
           <version>${plugin.scala.alchim31.version}</version>
+          <configuration>

Review Comment:
   I removed redundant configuration only in this PR. I, however, feel like we'd better clean up scala configuration. @Reamer @zjffdu Maven option is only for building Zeppelin itself, not running it, so I think we could simplify the scala compile option. I created https://issues.apache.org/jira/browse/ZEPPELIN-5864



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer commented on pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
Reamer commented on PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520#issuecomment-1344039188

   Please rebase your branch to the current master, PR #4534 has corrected core / interpreter-test-non-core.


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] jongyoul merged pull request #4520: [MINOR] Fix Appveyor error

Posted by GitBox <gi...@apache.org>.
jongyoul merged PR #4520:
URL: https://github.com/apache/zeppelin/pull/4520


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

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