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/10/26 03:46:53 UTC
[GitHub] [zeppelin] huage1994 opened a new pull request, #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
huage1994 opened a new pull request, #4498:
URL: https://github.com/apache/zeppelin/pull/4498
### What is this PR for?
This PR is to improve a some step in Frontend CI jobs.
There are two command in this step in file `frontend.yml` as follow:
https://github.com/apache/zeppelin/blob/f42943c6d5bc04498b7fc9ca518dc433091a9c9c/.github/workflows/frontend.yml#L123-L125
The first command would `mvn install` many modules including the submodule of `zeppelin-plugins`.
The second command would `mvn package` `zeppelin-plugins`.
### What type of PR is it?
Improvement
### Todos
* [ ] - Task
### What is the Jira issue?
* Open an issue on Jira [ZEPPELIN-5815](https://issues.apache.org/jira/browse/ZEPPELIN-5815)
### How should this be tested?
CI passed
### Screenshots (if appropriate)
### 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 a diff in pull request #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
Reamer commented on code in PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#discussion_r1007741613
##########
zeppelin-integration/pom.xml:
##########
@@ -94,21 +110,55 @@
</exclusions>
<scope>test</scope>
</dependency>
-
+
<dependency>
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-lang3</artifactId>
+ <groupId>org.apache.zeppelin</groupId>
+ <artifactId>spark-scala-2.11</artifactId>
Review Comment:
Depending on the spark-scala-2.11 or spark-scale-2.12 profile, one or the other dependency should be added.
##########
.github/workflows/frontend.yml:
##########
@@ -49,7 +49,7 @@ jobs:
restore-keys: |
${{ runner.os }}-zeppelin-
- name: Install application
- run: ./mvnw -B install -DskipTests -DskipRat -pl ${INTERPRETERS} -Phadoop2 -Pscala-2.11
+ run: ./mvnw clean install -pl zeppelin-web -B -Pintegration -Pspark-scala-2.11 -Pspark-2.4 -Phadoop2 -DskipTests -DskipRat -am
Review Comment:
You should use the same profiles that are used for testing. If the profiles are not sufficient for testing, they must be adjusted so that the profiles are the same during installation and testing.
The order of the Maven parameters should be the same for better overview. The parameter `-B` should be at the end. This simplifies the local testing very much.
##########
zeppelin-integration/pom.xml:
##########
@@ -94,21 +110,55 @@
</exclusions>
<scope>test</scope>
</dependency>
-
+
<dependency>
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-lang3</artifactId>
+ <groupId>org.apache.zeppelin</groupId>
+ <artifactId>spark-scala-2.11</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
</dependency>
<dependency>
- <groupId>org.rauschig</groupId>
- <artifactId>jarchivelib</artifactId>
- <version>0.7.1</version>
- <exclusions>
- <exclusion>
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-compress</artifactId>
- </exclusion>
- </exclusions>
+ <groupId>org.apache.zeppelin</groupId>
+ <artifactId>spark-shims</artifactId>
Review Comment:
Perhaps the `spark-interpreter` dependency is what is actually needed here.
##########
zeppelin-web/pom.xml:
##########
@@ -41,6 +41,51 @@
<plugin.frontend.npmDownloadRoot>https://registry.npmjs.org/npm/-/</plugin.frontend.npmDownloadRoot>
</properties>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.zeppelin</groupId>
+ <artifactId>spark-scala-2.11</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.zeppelin</groupId>
+ <artifactId>spark-shims</artifactId>
Review Comment:
Same as above.
##########
zeppelin-web/pom.xml:
##########
@@ -41,6 +41,51 @@
<plugin.frontend.npmDownloadRoot>https://registry.npmjs.org/npm/-/</plugin.frontend.npmDownloadRoot>
</properties>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.zeppelin</groupId>
+ <artifactId>spark-scala-2.11</artifactId>
Review Comment:
Same as above.
--
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 #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
Reamer commented on PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#issuecomment-1291985709
> But in the other frontend Job `run-e2e-tests-in-zeppelin-web`, I think it is inappropriate to add test dependencies in pom.xml of `zeppelin-web` , so I use `./mvnw clean package -pl zeppelin-integration -B -Pintegration -Pspark-scala-2.11 -Pspark-2.4 -Phadoop2 -DskipTests -DskipRat -am` to install as few modules as possible.
> How do you think about it?
You are moving in the right direction. That is exactly what I meant. You should add the `<scope>test</scope>` to your dependencies. In the end, it doesn't make any difference with this module, as it only consists of tests, but it's cleaner.
The CI test `run-e2e-tests-in-zeppelin-web` builds on the sub-module `zeppelin-web`. Also in the pom.xml all dependencies for the tests should be defined.
So that we only call the following commands in the workflow file.
`./mvnw clean install -pl zeppelin-web -B -Pscala-2.11 -Pweb-e2e -DskipTests -DskipRat -am` and afterwards for testing
`xvfb-run --auto-servernum --server-args="-screen 0 1024x768x24" ./mvnw verify -pl zeppelin-web -B -Pscala-2.11 -Pweb-e2e -DskipRat`
--
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 #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
jongyoul commented on code in PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#discussion_r1008831252
##########
.github/workflows/frontend.yml:
##########
@@ -49,9 +49,9 @@ jobs:
restore-keys: |
${{ runner.os }}-zeppelin-
- name: Install application
- run: ./mvnw -B install -DskipTests -DskipRat -pl ${INTERPRETERS} -Phadoop2 -Pscala-2.11
+ run: ./mvnw clean install -DskipTests -DskipRat -am -pl zeppelin-web -Pscala-2.11 -Pspark-scala-2.11 -Pspark-2.4 -Phadoop2 -Pweb-e2e -Pintegration -B
Review Comment:
```suggestion
run: ./mvnw clean install -DskipTests -DskipRat -am -pl zeppelin-web -Pscala-2.11 -Pspark-scala-2.11 -Pspark-2.4 -Phadoop2 -Pweb-e2e -Pintegration -B
```
--
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] huage1994 commented on pull request #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
huage1994 commented on PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#issuecomment-1294532929
The CI seems to run well.
I would appreciate it if anyone help review 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] huage1994 commented on pull request #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
huage1994 commented on PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#issuecomment-1296597025
Thanks a lot for your review @Reamer @jongyoul .
[Now the CI seems OK in my forked repo. ](https://github.com/huage1994/zeppelin/actions/runs/3355638130)
--
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 #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
Reamer commented on PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#issuecomment-1296772904
I have opened a small PR to use the scala profile. https://github.com/huage1994/zeppelin/pull/1
Let's wait for the CI.
--
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] huage1994 commented on pull request #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
huage1994 commented on PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#issuecomment-1291561298
> Sorry if I was vague here. Let me describe it in a little more detail. During the tests the project `zeppelin-integration` is executed. In my opinion this sub-module has to define all dependencies in the `pom.xml` so that the tests can be executed successfully, without install the whole zeppelin project via `mvn install` before. Unfortunately this is not yet the case, because only a few dependencies are needed during the installation.
>
> ```
> ./mvnw package -pl zeppelin-integration -Pintegration -am -DskipTests
> [INFO] Scanning for projects...
> [WARNING] The project org.apache.zeppelin:zeppelin-integration:jar:0.11.0-SNAPSHOT uses prerequisites which is only intended for maven-plugin projects but not for non maven-plugin projects. For such purposes you should use the maven-enforcer-plugin. See https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Build Order:
> [INFO]
> [INFO] Zeppelin [pom]
> [INFO] Zeppelin: Common [jar]
> [INFO] Zeppelin: Interpreter [jar]
> [INFO] Zeppelin: Jupyter Support [jar]
> [INFO] Zeppelin: Zengine [jar]
> [INFO] Zeppelin: Integration Test [jar]
> ```
>
> My point is that it may not be necessary to install the Zeppelin plugins at all.
Hi @Reamer , thanks a lot for your advice. It took me a while to get to know the module `zeppelin-integration` and I really learned a lot.
I've define all dependencies in the pom.xml of submodule `zeppelin-integration`. It works well in frontend job `test-selenium-with-spark-module-for-spark-2-4`
But in the other frontend Job `run-e2e-tests-in-zeppelin-web`, I think it is inappropriate to add test dependencies in pom.xml of `zeppelin-web` . I still use `./mvnw clean package -pl zeppelin-integration -B -Pintegration -Pspark-scala-2.11 -Pspark-2.4 -Phadoop2 -DskipTests -DskipRat -am` to install as few modules as possible.
How do you think about 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] jongyoul merged pull request #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
jongyoul merged PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498
--
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] huage1994 commented on pull request #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
huage1994 commented on PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#issuecomment-1292001355
> > But in the other frontend Job `run-e2e-tests-in-zeppelin-web`, I think it is inappropriate to add test dependencies in pom.xml of `zeppelin-web` , so I use `./mvnw clean package -pl zeppelin-integration -B -Pintegration -Pspark-scala-2.11 -Pspark-2.4 -Phadoop2 -DskipTests -DskipRat -am` to install as few modules as possible.
> > How do you think about it?
>
> You are moving in the right direction. That is exactly what I meant. You should add the `<scope>test</scope>` to your dependencies. In the end, it doesn't make any difference with this module, as it only consists of tests, but it's cleaner.
>
> The CI test `run-e2e-tests-in-zeppelin-web` builds on the sub-module `zeppelin-web`. Also in the pom.xml all dependencies for the tests should be defined. So that we only call the following commands in the workflow file. `./mvnw clean install -pl zeppelin-web -B -Pscala-2.11 -Pweb-e2e -DskipTests -DskipRat -am` and afterwards for testing `xvfb-run --auto-servernum --server-args="-screen 0 1024x768x24" ./mvnw verify -pl zeppelin-web -B -Pscala-2.11 -Pweb-e2e -DskipRat`
Thx! I get 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] huage1994 commented on pull request #4498: [ZEPPELIN-5815] Frontend CI jobs improvement
Posted by GitBox <gi...@apache.org>.
huage1994 commented on PR #4498:
URL: https://github.com/apache/zeppelin/pull/4498#issuecomment-1291459698
The code branch of the previous PR #4459 was accidentally deleted by me, so it cannot be reopen now. But you can see the past discussion #4459.
--
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