You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "maobaolong (via GitHub)" <gi...@apache.org> on 2024/07/26 01:19:04 UTC
[PR] [MINOR] Move rss-env.sh to conf folder [incubator-uniffle]
maobaolong opened a new pull request, #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): support xxx"
- "[#233] fix: check null before access result in xxx"
- "[MINOR] refactor: fix typo in variable name"
- "[MINOR] docs: fix typo in README"
- "[#255] test: fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. Contributor guidelines:
https://github.com/apache/incubator-uniffle/blob/master/CONTRIBUTING.md
3. If the PR is unfinished, please mark this PR as draft.
-->
### What changes were proposed in this pull request?
Move rss-env.sh to conf folder.
### Why are the changes needed?
`rss-env.sh` is a part of config file, it is mutable, we should maintain it within config server rather than `bin` folder
### Does this PR introduce _any_ user-facing change?
(Please list the user-facing changes introduced by your change, including
1. Change in user-facing APIs.
2. Addition or removal of property keys.)
`rss-env.sh` is moved to `conf` folder.
### How was this patch tested?
Start all components successfully.
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1966] improvement(script): Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#issuecomment-2252105751
@rickyma @advancedxy Thanks for your 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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [MINOR] Improvement(script) Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#discussion_r1692418510
##########
bin/utils.sh:
##########
@@ -159,6 +159,12 @@ function load_rss_env {
# find rss-env.sh
set +o nounset
+ if [ -z "$RSS_HOME" ]; then
+ RSS_HOME="$(cd "$(dirname "$0")/.."; pwd)"
+ fi
+ if [ -z "$RSS_CONF_DIR" ]; then
+ RSS_CONF_DIR="${RSS_HOME}/conf"
+ fi
Review Comment:
I'm wondering the implication of these changes.
If RSS_HOME/RSS_CONF_DIR is both set in rss-env.sh and manually set by user when starting the scripts, there's a chance that they are different, and the settings in rss-env.sh take preference over manually set by user. I think that might be confusing and error-prone. Let's add some clarification in the rss-env.sh, something like:
```shell
# RSS_HOME, RSS home directory (Default: parent directory of the script).
# If you want to set it to another place, please make sure the RSS_HOME
# specified externally is respected, by defining it as follows:
#
# RSS_HOME=${RSS_HOME:-{another_rss_home_path}}
#
# RSS_CONF_DIR, RSS configuration directory (Default: ${RSS_HOME}/conf)
# Similar with RSS_HOME, RSS_CONF_DIR should respect external env variable.
#
# RSS_CONF_DIR=${RSS_CONF_DIR:-{another_rss_conf_dir}}
```
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1966] Improvement(script) Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#issuecomment-2252089981
> This is not minor. Please create an issue to track this.
Done
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1966] improvement(script): Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma merged PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [MINOR] Improvement(script) Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#issuecomment-2251797492
## Test Results
2 732 files +10 2 732 suites +10 5h 38m 39s :stopwatch: + 4m 46s
969 tests ± 0 968 :white_check_mark: + 1 1 :zzz: ±0 0 :x: ±0
12 144 runs +10 12 129 :white_check_mark: +11 15 :zzz: ±0 0 :x: ±0
Results for commit d7a6c733. ± Comparison against base commit c3d8a804.
[test-results]:data:application/gzip;base64,H4sIAEb/omYC/03Myw6DIBCF4VcxrLuQq05fphkHTEi9NAgr03cvGqQuzzeTf2ejn9zGno3opHg0bEs+3rdNAaNfl0NaySFTvsfjA0xdry0RndT/6e0/mXiFEf2Uoa3gQlhDkZCWo8kFV6rMK5pNQLVS1Rfcque+R2mdZx/zYLZDQ52UZhAEo0KEHiwO1JPVyAcHRirllGbfH7xr0E0SAQAA
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [MINOR] Improvement(script) Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#issuecomment-2252062654
@advancedxy Thanks for your suggested change, I pasted it into rss-env.sh, PTAL.
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [MINOR] Improvement(script) Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#issuecomment-2252078649
This is not minor. Please create an issue to track 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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [MINOR] Improvement(script) Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#issuecomment-2251781887
@advancedxy This is a single purpose PR, it just move `rss-env.sh` to conf folder, PTAL
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1966] improvement(script): Move rss-env.sh to conf folder [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on PR #1962:
URL: https://github.com/apache/incubator-uniffle/pull/1962#issuecomment-2252100293
Merged to master.
Thanks for your contribution. @maobaolong
Thanks for your review. @advancedxy
--
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@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org