You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2023/09/14 12:31:03 UTC
[GitHub] [incubator-celeborn] cxzl25 opened a new pull request, #1914: [CELEBORN-978] Improve dependency.sh replacement mode
cxzl25 opened a new pull request, #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914
### What changes were proposed in this pull request?
### Why are the changes needed?
When executing the update script locally, it may generate such a Log, which causes awk to exit with an error.
```
Downloading from nexus: httpxxxx
```
```bash
./dev/dependencies.sh --replace
```
```
awk: trying to access out of range field -1
input record number 1, file
source line number 2
```
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cfmcgrady commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1326756786
##########
dev/dependencies.sh:
##########
@@ -40,7 +40,8 @@ function mvn_build_classpath() {
$MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
$MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
grep -v "INFO\|WARN" | \
- # This will skip the first two lines
+ grep -v "Downloading from" | \
Review Comment:
> so use grep -A1
seems to be a simpler solution than `sbt_build_client_classpath`
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cxzl25 commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1326762544
##########
dev/dependencies.sh:
##########
@@ -39,8 +39,10 @@ DEP=""
function mvn_build_classpath() {
$MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
$MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
- grep -v "INFO\|WARN" | \
- # This will skip the first two lines
+ grep -A1 "Dependencies classpath:" | \
+ grep -v "^--$" | \
+ grep -v "Dependencies classpath:" | \
+ # This will skip the first two lines
tail -n +3 | \
Review Comment:
Looks like it can be removed.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] codecov[bot] commented on pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#issuecomment-1719381525
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1914?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#1914](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1914?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ffd3780) into [main](https://app.codecov.io/gh/apache/incubator-celeborn/commit/03fc00e6a6177da3a6c8205e4b47551d4c791ca6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (03fc00e) will **increase** coverage by `0.03%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #1914 +/- ##
==========================================
+ Coverage 46.58% 46.61% +0.03%
==========================================
Files 164 164
Lines 10247 10247
Branches 937 937
==========================================
+ Hits 4773 4776 +3
+ Misses 5160 5158 -2
+ Partials 314 313 -1
```
[see 1 file with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1914/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cxzl25 commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1326760556
##########
dev/dependencies.sh:
##########
@@ -40,7 +40,8 @@ function mvn_build_classpath() {
$MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
$MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
grep -v "INFO\|WARN" | \
- # This will skip the first two lines
+ grep -v "Downloading from" | \
Review Comment:
`awk` looks more professional and cool.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cxzl25 commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1325879829
##########
dev/dependencies.sh:
##########
@@ -186,6 +187,7 @@ case "$MODULE" in
SBT_PROJECT="celeborn-client-flink-1_17"
;;
*)
+ MODULE="server"
Review Comment:
Because the `MODULE` parameter is not required, `DEP` and `DEP_PR` may refer to the empty string `MODULE`.
##########
dev/dependencies.sh:
##########
@@ -37,10 +37,11 @@ DEP_PR=""
DEP=""
function mvn_build_classpath() {
- $MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
- $MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
Review Comment:
When the specified `MODULE` does not exist in the maven profile and it is an empty string, the command will become strange.
```bash
build/mvn -P clean install xx
```
If MODULE specifies a value, we will get a WARNING like this.
```
Warning: The requested profile "server" could not be activated because it does not exist.
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cfmcgrady commented on pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#issuecomment-1719889498
let's enable the dependencies check CI when we update the script `./dev/dependencies.sh` to ensure the changes is correct.
add `dev/dependencies.sh` for `paths`
https://github.com/apache/incubator-celeborn/blob/03fc00e6a6177da3a6c8205e4b47551d4c791ca6/.github/workflows/deps.yml#L20-L28
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cxzl25 commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1326750985
##########
dev/dependencies.sh:
##########
@@ -40,7 +40,8 @@ function mvn_build_classpath() {
$MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
$MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
grep -v "INFO\|WARN" | \
- # This will skip the first two lines
+ grep -v "Downloading from" | \
Review Comment:
Checked the `maven-dependency-plugin` plugin code, the output will change to a new line, so use `grep -A1`
https://github.com/apache/maven-dependency-plugin/blob/34348ecee0a3588037287df2cad9269e10975c40/src/main/java/org/apache/maven/plugins/dependency/fromDependencies/BuildClasspathMojo.java#L233
```java
getLog().info("Dependencies classpath:" + System.lineSeparator() + cpString);
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] waitinfuture closed pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture closed pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
URL: https://github.com/apache/incubator-celeborn/pull/1914
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cfmcgrady commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1326707438
##########
dev/dependencies.sh:
##########
@@ -40,7 +40,8 @@ function mvn_build_classpath() {
$MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
$MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
grep -v "INFO\|WARN" | \
- # This will skip the first two lines
+ grep -v "Downloading from" | \
Review Comment:
perhaps we should consider matching the keyword `[INFO] Dependencies classpath:` and extracting the second line using the `awk` command, as demonstrated in the `sbt_build_client_classpath` function, instead of using `grep -v`. WDYT?
https://github.com/apache/incubator-celeborn/blob/03fc00e6a6177da3a6c8205e4b47551d4c791ca6/dev/dependencies.sh#L60-L63
```shell
./build/mvn dependency:build-classpath -am -pl common
```
<details>
<summary>output</summary>
```
[INFO] --- maven-dependency-plugin:3.1.1:build-classpath (default-cli) @ celeborn-common_2.12 ---
[INFO] Dependencies classpath:
/home/chenfu/.m2/repository/org/slf4j/jcl-over-slf4j/1.7.36/jcl-over-slf4j-1.7.36.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-redis/4.1.93.Final/netty-codec-redis-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-native-epoll/4.1.93.Final/netty-transport-native-epoll-4.1.93.Final-linux-x86_64.jar:/home/chenfu/.m2/repository/org/apache/hadoop/hadoop-client-api/3.2.4/hadoop-client-api-3.2.4.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-dns/4.1.93.Final/netty-codec-dns-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-rxtx/4.1.93.Final/netty-transport-rxtx-4.1.93.Final.jar:/home/chenfu/.m2/repository/org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-stomp/4.1.93.Final/netty-codec-stomp-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-resolver-dns-native-macos/4.1.93.Final/netty-resolver-dns-native-macos-4.1.93.Final-osx-aarch_64.jar:/home/chenfu/.m2/repository/org/apache/
commons/commons-lang3/3.12.0/commons-lang3-3.12.0.jar:/home/chenfu/.m2/repository/io/netty/netty-transport/4.1.93.Final/netty-transport-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-resolver-dns-native-macos/4.1.93.Final/netty-resolver-dns-native-macos-4.1.93.Final-osx-x86_64.jar:/home/chenfu/.m2/repository/io/dropwizard/metrics/metrics-jvm/3.2.6/metrics-jvm-3.2.6.jar:/home/chenfu/.m2/repository/org/fusesource/leveldbjni/leveldbjni-all/1.8/leveldbjni-all-1.8.jar:/home/chenfu/.m2/repository/io/netty/netty-common/4.1.93.Final/netty-common-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-native-unix-common/4.1.93.Final/netty-transport-native-unix-common-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/dropwizard/metrics/metrics-core/3.2.6/metrics-core-3.2.6.jar:/home/chenfu/.m2/repository/org/apache/htrace/htrace-core4/4.1.0-incubating/htrace-core4-4.1.0-incubating.jar:/home/chenfu/.m2/repository/org/apache/commons/commons-crypto/1.0.0/commons-crypto-1
.0.0.jar:/home/chenfu/.m2/repository/io/netty/netty-handler-proxy/4.1.93.Final/netty-handler-proxy-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-xml/4.1.93.Final/netty-codec-xml-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-resolver/4.1.93.Final/netty-resolver-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-native-epoll/4.1.93.Final/netty-transport-native-epoll-4.1.93.Final-linux-aarch_64.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-socks/4.1.93.Final/netty-codec-socks-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-classes-kqueue/4.1.93.Final/netty-transport-classes-kqueue-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-native-kqueue/4.1.93.Final/netty-transport-native-kqueue-4.1.93.Final-osx-x86_64.jar:/home/chenfu/.m2/repository/org/roaringbitmap/RoaringBitmap/0.9.32/RoaringBitmap-0.9.32.jar:/home/chenfu/.m2/repository/com/google/code/findbugs/jsr305/1.3.9/jsr305-1.3.9.jar:
/home/chenfu/.m2/repository/io/netty/netty-codec/4.1.93.Final/netty-codec-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-classes-epoll/4.1.93.Final/netty-transport-classes-epoll-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-mqtt/4.1.93.Final/netty-codec-mqtt-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-handler/4.1.93.Final/netty-handler-4.1.93.Final.jar:/home/chenfu/.m2/repository/org/apache/ratis/ratis-proto/2.5.1/ratis-proto-2.5.1.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-memcache/4.1.93.Final/netty-codec-memcache-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-resolver-dns/4.1.93.Final/netty-resolver-dns-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-sctp/4.1.93.Final/netty-transport-sctp-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/dropwizard/metrics/metrics-graphite/3.2.6/metrics-graphite-3.2.6.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-http/4.1.93.Final/netty-co
dec-http-4.1.93.Final.jar:/home/chenfu/.m2/repository/org/apache/ratis/ratis-common/2.5.1/ratis-common-2.5.1.jar:/home/chenfu/.m2/repository/org/slf4j/jul-to-slf4j/1.7.36/jul-to-slf4j-1.7.36.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-udt/4.1.93.Final/netty-transport-udt-4.1.93.Final.jar:/home/chenfu/.m2/repository/com/google/guava/guava/14.0.1/guava-14.0.1.jar:/home/chenfu/.m2/repository/org/apache/ratis/ratis-thirdparty-misc/1.0.4/ratis-thirdparty-misc-1.0.4.jar:/home/chenfu/.m2/repository/org/scala-lang/scala-library/2.12.15/scala-library-2.12.15.jar:/home/chenfu/.m2/repository/io/netty/netty-buffer/4.1.93.Final/netty-buffer-4.1.93.Final.jar:/home/chenfu/.m2/repository/com/google/protobuf/protobuf-java/3.19.2/protobuf-java-3.19.2.jar:/home/chenfu/.m2/repository/io/netty/netty-resolver-dns-classes-macos/4.1.93.Final/netty-resolver-dns-classes-macos-4.1.93.Final.jar:/home/chenfu/.m2/repository/org/apache/hadoop/hadoop-client-runtime/3.2.4/hadoop-client-runtime-3.2.4.ja
r:/home/chenfu/.m2/repository/org/apache/ratis/ratis-client/2.5.1/ratis-client-2.5.1.jar:/home/chenfu/.m2/repository/commons-io/commons-io/2.13.0/commons-io-2.13.0.jar:/home/chenfu/.m2/repository/commons-logging/commons-logging/1.1.3/commons-logging-1.1.3.jar:/home/chenfu/.m2/repository/io/netty/netty-transport-native-kqueue/4.1.93.Final/netty-transport-native-kqueue-4.1.93.Final-osx-aarch_64.jar:/home/chenfu/.m2/repository/org/yaml/snakeyaml/1.33/snakeyaml-1.33.jar:/home/chenfu/.m2/repository/io/netty/netty-all/4.1.93.Final/netty-all-4.1.93.Final.jar:/home/chenfu/.m2/repository/org/scala-lang/scala-reflect/2.12.15/scala-reflect-2.12.15.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-smtp/4.1.93.Final/netty-codec-smtp-4.1.93.Final.jar:/home/chenfu/.m2/repository/org/roaringbitmap/shims/0.9.32/shims-0.9.32.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-http2/4.1.93.Final/netty-codec-http2-4.1.93.Final.jar:/home/chenfu/.m2/repository/io/netty/netty-codec-haproxy/4.1.93.Fina
l/netty-codec-haproxy-4.1.93.Final.jar
```
</details>
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cxzl25 commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1326752048
##########
dev/dependencies.sh:
##########
@@ -39,8 +39,10 @@ DEP=""
function mvn_build_classpath() {
$MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
$MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
- grep -v "INFO\|WARN" | \
- # This will skip the first two lines
+ grep -A1 "Dependencies classpath:" | \
+ grep -v "^--$" | \
Review Comment:
GNU grep support
```
--group-separator=SEP
Use SEP as a group separator. By default SEP is double hyphen (--).
--no-group-separator
Use empty string as a group separator.
```
But BSD grep does not support `--no-group-separator`
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] cfmcgrady commented on a diff in pull request #1914: [CELEBORN-978] Improve dependency.sh replacement mode
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1914:
URL: https://github.com/apache/incubator-celeborn/pull/1914#discussion_r1326760860
##########
dev/dependencies.sh:
##########
@@ -39,8 +39,10 @@ DEP=""
function mvn_build_classpath() {
$MVN -P$MODULE clean install -DskipTests -am -pl $MVN_MODULES
$MVN -P$MODULE dependency:build-classpath -am -pl $MVN_MODULES | \
- grep -v "INFO\|WARN" | \
- # This will skip the first two lines
+ grep -A1 "Dependencies classpath:" | \
+ grep -v "^--$" | \
+ grep -v "Dependencies classpath:" | \
+ # This will skip the first two lines
tail -n +3 | \
Review Comment:
after we used `grep -A1`, we don't need this skip now?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org