Skip to content

[DS-6091][standalone] Fix DolphinPluginDiscovery load plugin fails on Windows os#6207

Closed
LiuBodong wants to merge 6 commits intoapache:devfrom
LiuBodong:6091-bugfix-standalone-not-works-on-windows
Closed

[DS-6091][standalone] Fix DolphinPluginDiscovery load plugin fails on Windows os#6207
LiuBodong wants to merge 6 commits intoapache:devfrom
LiuBodong:6091-bugfix-standalone-not-works-on-windows

Conversation

@LiuBodong
Copy link
Contributor

@LiuBodong LiuBodong commented Sep 14, 2021

Purpose of the pull request

Fix the bug that StandaloneServer run failed on Windows system.

Brief change log

  • Fix bug when use "/target/classes" to judge path will always return false.

The bug's reason is use File#getPath.endsWith("/target/classes") to judge if the given path is compiled class path. On Windows, the delimiter of path is '', so the method always returns false.

The solution is use Paths.endsWith(Path other) method to judge, it will work correctly.

resolves #6091

Fix the bug that StandaloneServer run failed on Windows system.

(1) StandaloneServer fails on Windows

The bug's reason is when we call `Class#getProtectionDomain().getCodeSource().getLocation().getPath()` on Windows, JAVA will return a path starts with '/', and then we use `Paths.get(path)` to transform the path, the bug appears.

The solution is use `Paths.get(URL url)` method to transform the path, it will work correctly.

(2) DolphinPluginDiscovery fails on Windows

The bug's reason is use `File#getPath.endsWith("/target/classes")` to judge if the given path is compiled class path. On Windows, the delimiter of path is '\', so the method always returns false.

The solution is use `Paths.endsWith(Path other)` method to judge, it will work correctly.
StandaloneServer.class.getProtectionDomain().getCodeSource().getLocation().getPath(),
"../../../dolphinscheduler-alert-plugin/dolphinscheduler-alert-email/pom.xml"
).toAbsolutePath();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

StandaloneServer.class.getProtectionDomain().getCodeSource().getLocation().getPath(),
"../../../dolphinscheduler-task-plugin/dolphinscheduler-task-shell/pom.xml"
).toAbsolutePath();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@kezhenxu94
Copy link
Member

This PR is duplicate with #6101, can you please coordinate with @QuakeWang which PR we should continue?

@LiuBodong
Copy link
Contributor Author

This PR is duplicate with #6101, can you please coordinate with @QuakeWang which PR we should continue?

I already send an e-mail to @QuakeWang , we will talk later

@QuakeWang
Copy link
Contributor

We have discussed and come up with the following recommendation: This pr retains the changes to DolphinPluginDiscovery. The changes to StandaloneServer are still based on #6101.

liubodong added 2 commits September 17, 2021 08:33
… 6091-bugfix-standalone-not-works-on-windows
…tion and only retain modification of DolphinPluginDiscovery.java
@LiuBodong
Copy link
Contributor Author

I just revert the modification of StandaloneServer.java, now the pr only contains modification of DolphinPluginDiscovery.java. Please do a code review~

@kezhenxu94
Copy link
Member

@LiuBodong please update the PR title and commit message accordingly

@LiuBodong LiuBodong changed the title [DS-6091][standalone] fix StandaloneServer run failed on Windows system [DS-6091][standalone] Fix DolphinPluginDiscovery load plugin fails on Windows os Sep 17, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #6207 (a129b71) into dev (a1e447d) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #6207      +/-   ##
============================================
- Coverage     39.07%   39.06%   -0.01%     
+ Complexity     3250     3249       -1     
============================================
  Files           638      638              
  Lines         25266    25267       +1     
  Branches       2743     2743              
============================================
- Hits           9872     9871       -1     
- Misses        14516    14517       +1     
- Partials        878      879       +1     
Impacted Files Coverage Δ
...inscheduler/spi/plugin/DolphinPluginDiscovery.java 0.00% <0.00%> (ø)
...er/master/dispatch/host/assign/RandomSelector.java 77.77% <0.00%> (-5.56%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1e447d...a129b71. Read the comment docs.

@LiuBodong LiuBodong closed this Sep 17, 2021
@LiuBodong LiuBodong deleted the 6091-bugfix-standalone-not-works-on-windows branch September 17, 2021 09:21
@LiuBodong LiuBodong restored the 6091-bugfix-standalone-not-works-on-windows branch September 17, 2021 09:24
@LiuBodong LiuBodong reopened this Sep 17, 2021
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@LiuBodong LiuBodong closed this Nov 5, 2021
@LiuBodong
Copy link
Contributor Author

already fixed in another pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][dolphinscheduler-standalone-server] StandaloneServer run failed on Windows system

4 participants