Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement][Worker] Startup parameter should have the highest priority #13274

Merged
merged 5 commits into from
Dec 29, 2022

Conversation

Radeity
Copy link
Member

@Radeity Radeity commented Dec 25, 2022

Purpose of the pull request

Brief change log

  • Set startup parameter the highest priority.
  • Support convert time function in the middle of String.
  • Fix bug: The following situation will cause problem: if a local parameter and global parameter have the same name dx, another parameter(local or global) dy set the value ${dx}aaa, dx will be replaced by global parameter which has the lower priority. In addition, in every task, parameter placeholders will be replaced, either. Like in shell task:
    return ParameterUtils.convertParameterPlaceholders(script, ParamUtils.convert(paramsMap));
    Thus, remove parameter place holder conversion in paramParsingPreparation.

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@codecov-commenter
Copy link

codecov-commenter commented Dec 25, 2022

Codecov Report

Merging #13274 (9b9816d) into dev (8870464) will decrease coverage by 0.02%.
The diff coverage is 40.00%.

@@             Coverage Diff              @@
##                dev   #13274      +/-   ##
============================================
- Coverage     39.53%   39.51%   -0.03%     
+ Complexity     4293     4288       -5     
============================================
  Files          1072     1072              
  Lines         40558    40551       -7     
  Branches       4670     4668       -2     
============================================
- Hits          16035    16022      -13     
- Misses        22741    22746       +5     
- Partials       1782     1783       +1     
Impacted Files Coverage Δ
...inscheduler/service/expand/CuringGlobalParams.java 28.88% <40.00%> (-2.23%) ⬇️
...erver/master/processor/queue/TaskEventService.java 75.00% <0.00%> (-5.36%) ⬇️
...uler/api/controller/ProcessInstanceController.java 68.18% <0.00%> (-2.04%) ⬇️
...dolphinscheduler/remote/future/ResponseFuture.java 81.96% <0.00%> (-1.64%) ⬇️
...java/org/apache/dolphinscheduler/dao/AlertDao.java 28.46% <0.00%> (-0.86%) ⬇️
...pache/dolphinscheduler/common/utils/DateUtils.java 68.30% <0.00%> (-0.55%) ⬇️
...inscheduler/service/alert/ProcessAlertManager.java 41.70% <0.00%> (-0.38%) ⬇️
...heduler/api/service/impl/SchedulerServiceImpl.java 26.43% <0.00%> (-0.34%) ⬇️
...heduler/api/service/impl/ResourcesServiceImpl.java 38.54% <0.00%> (-0.12%) ⬇️
... and 7 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

brave-lee
brave-lee previously approved these changes Dec 26, 2022
Copy link
Contributor

@brave-lee brave-lee left a comment

Choose a reason for hiding this comment

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

+1

@sonarcloud
Copy link

sonarcloud bot commented Dec 28, 2022

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

36.4% 36.4% Coverage
0.0% 0.0% Duplication

@EricGao888 EricGao888 added this to the 3.2.0 milestone Dec 28, 2022
@EricGao888 EricGao888 added bug Something isn't working improvement make more easy to user or prompt friendly labels Dec 28, 2022
Copy link
Member

@EricGao888 EricGao888 left a comment

Choose a reason for hiding this comment

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

LGTM. Would u like to double check when available? Thanks : ) @JinyLeeChina @caishunfeng @zhongjiajie

@zhongjiajie zhongjiajie added the 3.2.0 for 3.2.0 version label Dec 29, 2022
@zhongjiajie zhongjiajie merged commit 8503ee0 into apache:dev Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.2.0 for 3.2.0 version backend bug Something isn't working document improvement make more easy to user or prompt friendly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement][Worker] Startup parameter should have the highest priority
5 participants