You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "chn5944 (via GitHub)" <gi...@apache.org> on 2024/12/04 13:05:52 UTC
[PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
chn5944 opened a new pull request, #25739:
URL: https://github.com/apache/flink/pull/25739
edit "flink-dist\src\main\flink-bin\bin\bash-java-utils.sh" to solve bug(start script can not get config for "env.java.home" in "config.yaml" correctly since 1.20)
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #25739:
URL: https://github.com/apache/flink/pull/25739#discussion_r1869668612
##########
flink-dist/src/main/flink-bin/bin/bash-java-utils.sh:
##########
@@ -25,11 +34,41 @@ readFromConfigFile() {
# first extract the value with the given key (1st sed), then trim the result (2nd sed)
# if a key exists multiple times, take the "last" one (tail)
local value=`sed -n "s/^[ ]*${key}[ ]*: \([^#]*\).*$/\1/p" "${configFile}" | sed "s/^ *//;s/ *$//" | tail -n 1`
-
+ if [ -z "$value" ]; then
+ # for env.java.home value = `sed -n "/^env:/,/^[^ ]/p" "${configFile}" | sed -n "/^ java:/,/^[^ ]/p" | sed -n "/home:/s/.*: //p" | tail -n 1`
Review Comment:
what is this commented out line for?
##########
flink-dist/src/main/flink-bin/bin/bash-java-utils.sh:
##########
@@ -25,11 +34,41 @@ readFromConfigFile() {
# first extract the value with the given key (1st sed), then trim the result (2nd sed)
# if a key exists multiple times, take the "last" one (tail)
local value=`sed -n "s/^[ ]*${key}[ ]*: \([^#]*\).*$/\1/p" "${configFile}" | sed "s/^ *//;s/ *$//" | tail -n 1`
-
+ if [ -z "$value" ]; then
Review Comment:
is there any way to add tests for this new logic?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2788405796
This PR is being marked as stale since it has not had any activity in the last 90 days.
If you would like to keep this PR alive, please leave a comment asking for a review.
If the PR has merge conflicts, update it with the latest from the base branch.
If you are having difficulty finding a reviewer, please reach out to the
community, contact details can be found here: https://flink.apache.org/what-is-flink/community/
If this PR is no longer valid or desired, please feel free to close it.
If no activity occurs in the next 30 days, it will be automatically closed.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "chn5944 (via GitHub)" <gi...@apache.org>.
chn5944 commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2518020325
> > @fsk119 i already report this issue by e-mail and get response from [fskmine@gmail.com](mailto:fskmine@gmail.com)
>
> @chn5944 I am curious what the email exchange was and whether it aligns with the current proposed fix?
I request a jira account which name is "fpy5944", but I haven't receive any email for jira acount status, so I send email to user-zh, this email owner reply me direct to submit pr and @ fsk119, then you will review it when you have 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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "chn5944 (via GitHub)" <gi...@apache.org>.
chn5944 commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2517302132
@fsk119 i already report this issue by e-mail and get response from fskmine@gmail.com
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #25739:
URL: https://github.com/apache/flink/pull/25739#discussion_r1869672924
##########
flink-dist/src/main/flink-bin/bin/bash-java-utils.sh:
##########
@@ -25,11 +34,41 @@ readFromConfigFile() {
# first extract the value with the given key (1st sed), then trim the result (2nd sed)
# if a key exists multiple times, take the "last" one (tail)
local value=`sed -n "s/^[ ]*${key}[ ]*: \([^#]*\).*$/\1/p" "${configFile}" | sed "s/^ *//;s/ *$//" | tail -n 1`
-
+ if [ -z "$value" ]; then
+ # for env.java.home value = `sed -n "/^env:/,/^[^ ]/p" "${configFile}" | sed -n "/^ java:/,/^[^ ]/p" | sed -n "/home:/s/.*: //p" | tail -n 1`
+ IFS='.'
+ read -ra nodes <<< "$1"
+
+ local sed_cmd_left="sed -n \"/^"
+ local sed_cmd_right=" | "
+ for index in ${!nodes[@]}; do
+ node=${nodes[index]}
+
+ if [ $index -eq 0 ]; then
Review Comment:
I suggest adding more comments explaining the intent of the code; it would be easier to parse for the reader
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "chn5944 (via GitHub)" <gi...@apache.org>.
chn5944 commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2519645304
I agree with @fsk119 and @JunRuiLee 's point, I would like to spend some time to check whether this fix can cover scenarios
which mixed with nested keys and flattened keys, and I expect better way to solve this
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2520052327
In the [Strimzi](https://github.com/strimzi/strimzi-kafka-operator) project we have had success using [`yq`](https://github.com/mikefarah/yq). I think to cover all the possible permutation, you would probably end up writing a tool like that in bash functions anyway, so may be we could add that as a dependency?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "chn5944 (via GitHub)" <gi...@apache.org>.
chn5944 commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2520882875
> before second commit I test "env.java.home" written both nested format with flattened format and mixed with other keys, template file is "config.yaml" in "flink-dist/src/test/resources/", current logic is flattened format prior to nested format when same key configure in the form of flattened format and nested format respectively。when key was configured in just one format, nested format or flattened format can be get correctly
I also think bash is not a good way to parse ".yaml" even not in mix format。 before I commit this PR, I test configue key in flattened format and comment this line with '#', setJavaHome function still parse this commented line, of course this is not a normal behavior
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2517304214
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "058187c119aab93b456a5f6846850112b14a20f4",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "058187c119aab93b456a5f6846850112b14a20f4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 058187c119aab93b456a5f6846850112b14a20f4 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "chn5944 (via GitHub)" <gi...@apache.org>.
chn5944 commented on code in PR #25739:
URL: https://github.com/apache/flink/pull/25739#discussion_r1869702019
##########
flink-dist/src/main/flink-bin/bin/bash-java-utils.sh:
##########
@@ -25,11 +34,41 @@ readFromConfigFile() {
# first extract the value with the given key (1st sed), then trim the result (2nd sed)
# if a key exists multiple times, take the "last" one (tail)
local value=`sed -n "s/^[ ]*${key}[ ]*: \([^#]*\).*$/\1/p" "${configFile}" | sed "s/^ *//;s/ *$//" | tail -n 1`
-
+ if [ -z "$value" ]; then
+ # for env.java.home value = `sed -n "/^env:/,/^[^ ]/p" "${configFile}" | sed -n "/^ java:/,/^[^ ]/p" | sed -n "/home:/s/.*: //p" | tail -n 1`
+ IFS='.'
+ read -ra nodes <<< "$1"
+
+ local sed_cmd_left="sed -n \"/^"
+ local sed_cmd_right=" | "
+ for index in ${!nodes[@]}; do
+ node=${nodes[index]}
+
+ if [ $index -eq 0 ]; then
Review Comment:
get it, I will follow your above suggest; I have no good idea to add tests for this now, I just test new logic is adaptive for "flink-conf.yaml" and "config.yaml", I will try add to tests, also looking forward to your good idea
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2520391303
Reviewed by Chi on 05/12/24. Asked submitter questions.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "chn5944 (via GitHub)" <gi...@apache.org>.
chn5944 commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2520846199
before second commit I test "env.java.home" written both nested format with flattened format and mixed with other keys, template file is "config.yaml" in "flink-dist/src/test/resources/", current logic is flattened format prior to nested format when same key configure in the form of flattened format and nested format respectively。when key was configured in just one format,
nested format or flattened format can be get correctly
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #25739:
URL: https://github.com/apache/flink/pull/25739#discussion_r1869670605
##########
flink-dist/src/main/flink-bin/bin/bash-java-utils.sh:
##########
@@ -25,11 +34,41 @@ readFromConfigFile() {
# first extract the value with the given key (1st sed), then trim the result (2nd sed)
# if a key exists multiple times, take the "last" one (tail)
local value=`sed -n "s/^[ ]*${key}[ ]*: \([^#]*\).*$/\1/p" "${configFile}" | sed "s/^ *//;s/ *$//" | tail -n 1`
-
+ if [ -z "$value" ]; then
+ # for env.java.home value = `sed -n "/^env:/,/^[^ ]/p" "${configFile}" | sed -n "/^ java:/,/^[^ ]/p" | sed -n "/home:/s/.*: //p" | tail -n 1`
+ IFS='.'
+ read -ra nodes <<< "$1"
+
+ local sed_cmd_left="sed -n \"/^"
+ local sed_cmd_right=" | "
+ for index in ${!nodes[@]}; do
+ node=${nodes[index]}
+
+ if [ $index -eq 0 ]; then
+ # echo "first node：$node"
Review Comment:
I suggest removing these commented out debug lines
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2517947100
> @fsk119 i already report this issue by e-mail and get response from [fskmine@gmail.com](mailto:fskmine@gmail.com)
@chn5944 I am curious what the email exchange was and whether it aligns with the current proposed fix?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "fsk119 (via GitHub)" <gi...@apache.org>.
fsk119 commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2519508466
@chn5944 Thanks a lot for your fix!
+1 with @JunRuiLee 's point. I think we can use other language to process this situation. I think we should read more design from other community to get a better solution to solve this problem.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25739:
URL: https://github.com/apache/flink/pull/25739#issuecomment-2865287430
This PR has been closed since it has not had any activity in 120 days.
If you feel like this was a mistake, or you would like to continue working on it,
please feel free to re-open the PR and ask for a review.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20 [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #25739: solve bash-java-utils.sh can't get "env.java.home" in "config.yaml" since 1.20
URL: https://github.com/apache/flink/pull/25739
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org