[Feature][Deploy] Add missing default values in dolphinscheduler_env.sh by EricGao888 · Pull Request #9733 · apache/dolphinscheduler

EricGao888 · 2022-04-24T18:20:32Z

Purpose of the pull request

Add missing default values in dolphinscheduler_env.sh
Update related docs
This PR closes: [Feature][Deploy] Add missing default values in dolphinscheduler_env.sh #9675

Brief change log

Already Described above

Verify this pull request

Verified by manual tests

codecov-commenter · 2022-04-24T18:35:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.06%. Comparing base (84cad13) to head (e9fe8a6).
⚠️ Report is 2453 commits behind head on dev.

Additional details and impacted files

@@             Coverage Diff              @@
##                dev    #9733      +/-   ##
============================================
- Coverage     40.07%   40.06%   -0.02%     
+ Complexity     4484     4481       -3     
============================================
  Files           835      835              
  Lines         33575    33587      +12     
  Branches       3712     3712              
============================================
+ Hits          13456    13457       +1     
- Misses        18865    18877      +12     
+ Partials       1254     1253       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

SbloodyS · 2022-04-26T01:52:32Z

Hi @EricGao888 , please resolve conficts.

EricGao888 · 2022-04-26T02:48:20Z

Hi @EricGao888 , please resolve conficts.

@SbloodyS Thx for the reminder! I'm a bit confused about what to do in this PR, since we no more need add default values to dolphinscheduler_env.sh as explained in this comment: #9675 (comment)

SbloodyS · 2022-04-26T04:45:04Z

@EricGao888 Finally, we have an agreement. please see #9675 (comment)

EricGao888 · 2022-04-26T07:43:02Z

@EricGao888 Finally, we have an agreement. please see #9675 (comment)

@SbloodyS I just checked the latest code, it seems this feature has been implemented by PR #9726

dolphinscheduler/script/dolphinscheduler-daemon.sh

Lines 37 to 51 in 6370287

    
           BIN_ENV_FILE="${DOLPHINSCHEDULER_HOME}/bin/env/dolphinscheduler_env.sh" 
        
           # Overwrite server dolphinscheduler_env.sh in path `<server>/conf/dolphinscheduler_env.sh` when exists 
        
           # `bin/env/dolphinscheduler_env.sh` file. User could only change `bin/env/dolphinscheduler_env.sh` instead 
        
           # of each server's dolphinscheduler_env.sh when they want to start the server 
        
           function overwrite_server_env() { 
        
             local server=$1 
        
             local server_env_file="${DOLPHINSCHEDULER_HOME}/${server}/conf/dolphinscheduler_env.sh" 
        
             if [ -f "${BIN_ENV_FILE}" ]; then 
        
               echo "Overwrite ${server}/conf/dolphinscheduler_env.sh using bin/env/dolphinscheduler_env.sh." 
        
               cp "${BIN_ENV_FILE}" "${server_env_file}" 
        
             else 
        
               echo "Start server ${server} using env config path ${server_env_file}, because file ${BIN_ENV_FILE} not exists." 
        
             fi 
        
           }

SbloodyS · 2022-04-26T07:46:36Z

Yes. But some of the default value in dolphinscheduler_env.sh still missing. @EricGao888

dolphinscheduler/script/env/dolphinscheduler_env.sh

Lines 24 to 27 in 6370287

    
           export SPRING_DATASOURCE_DRIVER_CLASS_NAME 
        
           export SPRING_DATASOURCE_URL 
        
           export SPRING_DATASOURCE_USERNAME 
        
           export SPRING_DATASOURCE_PASSWORD

EricGao888 · 2022-04-26T07:50:09Z

Yes. But some of the default value in dolphinscheduler_env.sh still missing. @EricGao888

dolphinscheduler/script/env/dolphinscheduler_env.sh

Lines 24 to 27 in 6370287

export SPRING_DATASOURCE_DRIVER_CLASS_NAME

export SPRING_DATASOURCE_URL

export SPRING_DATASOURCE_USERNAME

export SPRING_DATASOURCE_PASSWORD

Sure I will fix it.

…sh (apache#9675)

EricGao888 · 2022-04-26T12:03:55Z

I added missing default values in dolphinscheduler_env.sh, updated related docs and fixed minor issue in update-schema.sh following PR #9726, PTAL when you have time. Thx! @zhongjiajie @SbloodyS

BTW, not sure why some E2E tests failed.

SbloodyS · 2022-04-26T13:06:52Z

script/env/dolphinscheduler_env.sh will overwrite the default setting of stanalone server. Standalone server starts with H2 database by default. That's why E2E tests failed.

We may find a way bypass it. Otherwise, restore script/env/dolphinscheduler_env.sh as it is. @EricGao888

EricGao888 · 2022-04-26T13:35:26Z

script/env/dolphinscheduler_env.sh will overwrite the default setting of stanalone server. Standalone server starts with H2 database by default. That's why E2E tests failed.

We may find a way bypass it. Otherwise, restore script/env/dolphinscheduler_env.sh as it is. @EricGao888

@SbloodyS I think it may not be easy to find a way to bypass it. Considering we already have default values in application.yaml and also provider users with examples in docs in this PR, should we just restore script/env/dolphinscheduler_env.sh and leave those default values blank?

dolphinscheduler/dolphinscheduler-tools/src/main/resources/application.yaml

Lines 17 to 24 in 52b188b

    
           spring: 
        
             main: 
        
               banner-mode: off 
        
             datasource: 
        
               driver-class-name: org.postgresql.Driver 
        
               url: jdbc:postgresql://127.0.0.1:5432/dolphinscheduler 
        
               username: root 
        
               password: root

SbloodyS · 2022-04-26T13:42:00Z

@EricGao888 I think these examples is enough for users.

apache#9675)

EricGao888 · 2022-04-26T14:13:00Z

@EricGao888 I think these examples is enough for users.

Done, CI looks good now. : )

docs/docs/en/guide/installation/pseudo-cluster.md

….Driver (apache#9675)

SbloodyS

LGTM if CI passed. Do you have time to take a look? @zhongjiajie

zhongjiajie · 2022-04-27T01:55:57Z

I am take a look on this PR now

docs/docs/en/guide/installation/pseudo-cluster.md

zhongjiajie · 2022-04-27T02:08:11Z

dolphinscheduler-tools/src/main/bin/create-schema.sh

 DOLPHINSCHEDULER_HOME=${DOLPHINSCHEDULER_HOME:-$(cd $BIN_DIR/../..; pwd)}
-source "$DOLPHINSCHEDULER_HOME/tools/bin/dolphinscheduler_env.sh"
+
+source "$DOLPHINSCHEDULER_HOME/bin/env/dolphinscheduler_env.sh"


if we do not use dolphinscheduler_env.sh file anymore, we should also remove the assembly config

dolphinscheduler/dolphinscheduler-tools/src/main/assembly/dolphinscheduler-tools.xml

Lines 41 to 49 in de50f43

<fileSet>

<directory>${basedir}/../script/env</directory>

<outputDirectory>conf</outputDirectory>

<includes>

<include>dolphinscheduler_env.sh</include>

</includes>

<fileMode>0755</fileMode>

<directoryMode>0755</directoryMode>

</fileSet>

I think it is a good idea to remove the assembly config in case users get confused. Will fix it in the next commit. : )

@SbloodyS @zhongjiajie E2E tests failed. May I ask is it caused by the removal of assembly config? Is there any way to bypass it? Thx!

I do not think it will cause the failed. I will restart the CI

@EricGao888 It's E2E build failed. Maybe it's the problem with GitHub Actions since it can't guarantee 99.99% stability...

@EricGao888 It's E2E build failed. Maybe it's the problem with GitHub Actions since it can't guarantee 99.99% stability...

could I @github at this time？ 🤣

sonarqubecloud · 2022-04-27T03:35:46Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

zhongjiajie

LGTM, @SbloodyS do you have addition suggestion?

SbloodyS

+1

SbloodyS assigned EricGao888 Apr 26, 2022

SbloodyS added the document label Apr 26, 2022

EricGao888 force-pushed the Fix-9675 branch 2 times, most recently from af61f9f to 3567104 Compare April 26, 2022 11:05

[Feature][Deploy] Add missing default values in dolphinscheduler_env.…

79f708a

…sh (apache#9675)

EricGao888 force-pushed the Fix-9675 branch from 3567104 to 79f708a Compare April 26, 2022 11:13

[Feature][Deploy] Restore dolphinscheduler_env.sh to avoid e2e failure (

cbb7b69

apache#9675)

SbloodyS reviewed Apr 26, 2022

View reviewed changes

docs/docs/en/guide/installation/pseudo-cluster.md Outdated Show resolved Hide resolved

[Feature][Deploy] Change mysql driver class name to com.mysql.cj.jdbc…

06d8418

….Driver (apache#9675)

SbloodyS previously approved these changes Apr 26, 2022

View reviewed changes

zhongjiajie reviewed Apr 27, 2022

View reviewed changes

zhongjiajie added the Waiting for user feedback Waiting for feedback from issue/PR author label Apr 27, 2022

[Feature][Deploy] Remove redundant config file (apache#9675)

e9fe8a6

EricGao888 dismissed SbloodyS’s stale review via e9fe8a6 April 27, 2022 03:12

SbloodyS removed the Waiting for user feedback Waiting for feedback from issue/PR author label Apr 27, 2022

EricGao888 requested a review from zhongjiajie April 27, 2022 05:37

EricGao888 requested a review from SbloodyS April 27, 2022 05:37

zhongjiajie approved these changes Apr 27, 2022

View reviewed changes

SbloodyS approved these changes Apr 27, 2022

View reviewed changes

SbloodyS merged commit 515c363 into apache:dev Apr 27, 2022

EricGao888 deleted the Fix-9675 branch April 27, 2022 05:48

This was referenced May 15, 2022

[Bug] fix run on docker and k8s #10026

Merged

[Feature][script] Bash in tools/bin is not executable #9960

Closed

	<fileSet>
	<directory>${basedir}/../script/env</directory>
	<outputDirectory>conf</outputDirectory>
	<includes>
	<include>dolphinscheduler_env.sh</include>
	</includes>
	<fileMode>0755</fileMode>
	<directoryMode>0755</directoryMode>
	</fileSet>

Conversation

EricGao888 commented Apr 24, 2022

Purpose of the pull request

Brief change log

Verify this pull request

Uh oh!

codecov-commenter commented Apr 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

SbloodyS commented Apr 26, 2022

Uh oh!

EricGao888 commented Apr 26, 2022

Uh oh!

SbloodyS commented Apr 26, 2022

Uh oh!

EricGao888 commented Apr 26, 2022

Uh oh!

SbloodyS commented Apr 26, 2022

Uh oh!

EricGao888 commented Apr 26, 2022

Uh oh!

EricGao888 commented Apr 26, 2022

Uh oh!

SbloodyS commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricGao888 commented Apr 26, 2022

Uh oh!

SbloodyS commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricGao888 commented Apr 26, 2022

Uh oh!

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

zhongjiajie commented Apr 27, 2022

Uh oh!

Uh oh!

Uh oh!

zhongjiajie Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

zhongjiajie Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

SbloodyS Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

zhongjiajie Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 27, 2022

Uh oh!

zhongjiajie left a comment

Choose a reason for hiding this comment

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Apr 24, 2022 •

edited

Loading

SbloodyS commented Apr 26, 2022 •

edited

Loading

SbloodyS commented Apr 26, 2022 •

edited

Loading