Skip to content

[DSIP-39][parameter] Improvement startup parameters/global parameters/project parameters data type#15945

Closed
sdhzwc wants to merge 0 commit intoapache:devfrom
sdhzwc:dev
Closed

[DSIP-39][parameter] Improvement startup parameters/global parameters/project parameters data type#15945
sdhzwc wants to merge 0 commit intoapache:devfrom
sdhzwc:dev

Conversation

@sdhzwc
Copy link
Contributor

@sdhzwc sdhzwc commented Apr 30, 2024

close #15936

Purpose of the pull request

Brief change log

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

@github-actions github-actions bot added UI ui and front end related backend labels Apr 30, 2024
@SbloodyS SbloodyS added improvement make more easy to user or prompt friendly 3.3.0 labels Apr 30, 2024
@SbloodyS SbloodyS added this to the 3.3.0 milestone Apr 30, 2024
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

Need to discuss at #15936.

songjianet
songjianet previously approved these changes May 6, 2024
@sdhzwc sdhzwc changed the title [Improvement-15936] New data types and type filtering [DSIP][parameter] Improvement startup parameters/global parameters/project parameters data type May 8, 2024
@sdhzwc sdhzwc requested a review from ruanwenjun May 8, 2024 09:51
@codecov-commenter
Copy link

codecov-commenter commented May 9, 2024

Codecov Report

Attention: Patch coverage is 55.88235% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 39.93%. Comparing base (d01d3de) to head (665f48a).
Report is 1 commits behind head on dev.

❗ Current head 665f48a differs from pull request most recent head 95b5aa3. Consider uploading reports for the commit 95b5aa3 to get more accurate results

Files Patch % Lines
...dolphinscheduler/api/utils/DataTransformUtils.java 42.85% 7 Missing and 1 partial ⚠️
...eduler/service/expand/CuringParamsServiceImpl.java 69.23% 3 Missing and 1 partial ⚠️
...cheduler/api/service/impl/ExecutorServiceImpl.java 0.00% 1 Missing and 1 partial ⚠️
...inscheduler/api/controller/ExecutorController.java 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #15945      +/-   ##
============================================
+ Coverage     39.85%   39.93%   +0.07%     
- Complexity     5060     5082      +22     
============================================
  Files          1369     1370       +1     
  Lines         45659    45653       -6     
  Branches       4872     4870       -2     
============================================
+ Hits          18199    18232      +33     
+ Misses        25560    25521      -39     
  Partials       1900     1900              

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

@sdhzwc sdhzwc requested a review from ruanwenjun May 9, 2024 06:48
@sdhzwc sdhzwc changed the title [DSIP][parameter] Improvement startup parameters/global parameters/project parameters data type [DSIP-39][parameter] Improvement startup parameters/global parameters/project parameters data type May 9, 2024
Comment on lines 174 to 169
JsonElement jsonElement = JsonParser.parseString(startParams);
boolean isJson = jsonElement.isJsonObject();
if (isJson) {
Map<String, String> startParamMap = JSONUtils.toMap(startParams);
startParamList = startParamMap.entrySet().stream()
.map(entry -> new Property(entry.getKey(), Direct.IN, DataType.VARCHAR, entry.getValue()))
.collect(Collectors.toList());
} else {
startParamList = JSONUtils.toList(startParams, Property.class);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Add a utils to transform the json to Propertity list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a utils to transform the json to Propertity list

done

Comment on lines +282 to +291
JsonElement jsonElement = JsonParser.parseString(startParams);
boolean isJson = jsonElement.isJsonObject();
if (isJson) {
Map<String, String> startParamMap = JSONUtils.toMap(startParams);
startParamList = startParamMap.entrySet().stream()
.map(entry -> new Property(entry.getKey(), Direct.IN, DataType.VARCHAR, entry.getValue()))
.collect(Collectors.toList());
} else {
startParamList = JSONUtils.toList(startParams, Property.class);
}
Copy link
Member

Choose a reason for hiding this comment

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

same

Comment on lines +764 to +765
if (startParamList != null && startParamList.size() > 0) {
cmdParam.put(CMD_PARAM_START_PARAMS, JSONUtils.toJsonString(startParamList));
Copy link
Member

Choose a reason for hiding this comment

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

Use CollectionUtils.isNotEmpty(startParamList)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use CollectionUtils.isNotEmpty(startParamList)

done

@ruanwenjun ruanwenjun added 3.2.2 and removed 3.3.0 labels May 9, 2024
@ruanwenjun ruanwenjun modified the milestones: 3.3.0, 3.2.2 May 9, 2024
@ruanwenjun
Copy link
Member

Frontend compile failed.

@sdhzwc
Copy link
Contributor Author

sdhzwc commented May 9, 2024

Frontend compile failed.

done

@sdhzwc sdhzwc requested review from ruanwenjun and songjianet May 9, 2024 09:38
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@ruanwenjun ruanwenjun closed this May 9, 2024
@ruanwenjun
Copy link
Member

Sorry I push to your branch, but the pr auto closed, please reopen it.

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

Labels

3.2.2 backend improvement make more easy to user or prompt friendly UI ui and front end related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DSIP-39][parameter] Startup parameters/global parameters/project parameters support set data type

5 participants