[Fix-17943][dolphinscheduler-task-dinky] When DolphinScheduler calls a Dinky job and passes date parameters, the actual values are not being replaced#18080
Conversation
shaolei7788
requested review from
Gallardot,
SbloodyS and
caishunfeng
as code owners
SbloodyS
left a comment
SbloodyS
left a comment
There was a problem hiding this comment.
Please do not create multiple PRs repeatedly next time. This will lead to an increase in reviewer's work.
| } | ||
|
|
||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Revert unnessnary chanage.
ok
SbloodyS
added
bug
| Map<String, Property> prepareParamsMap = taskExecutionContext.getPrepareParamsMap(); | ||
| prepareParamsMap.forEach((key, property) -> { | ||
| if (property != null && property.getValue() != null) { | ||
| variables.put(key, property.getValue().trim()); | ||
| } | ||
| } | ||
| }); | ||
| List<Property> localParams = this.dinkyParameters.getLocalParams(); | ||
| Map<String, Property> prepareParamsMap = taskExecutionContext.getPrepareParamsMap(); | ||
| if (localParams == null || localParams.isEmpty()) { | ||
| return variables; | ||
| } | ||
| Map<String, String> convertMap = ParameterUtils.convert(prepareParamsMap); | ||
| for (Property property : localParams) { | ||
| String propertyValue = property.getValue(); | ||
| String value = PlaceholderUtils.replacePlaceholders(propertyValue, convertMap, true); | ||
| variables.put(property.getProp(), value); | ||
| if (localParams != null) { | ||
| for (Property property : localParams) { | ||
| String value = ParameterUtils.convertParameterPlaceholders(property.getValue(), variables); | ||
| if (value != null && !value.isEmpty()) { | ||
| variables.put(property.getProp(), value.trim()); | ||
| } | ||
| } | ||
| } | ||
| log.info("sending variables to dinky: {}", variables); |
There was a problem hiding this comment.
Maybe directly use taskExecutionContext.getPrepareParamsMap() is enough?
There was a problem hiding this comment.
Maybe directly use
taskExecutionContext.getPrepareParamsMap()is enough?
No,it is not enough. The time function is not converted.
There was a problem hiding this comment.
Are you sure? please test it on dev branch, this should be handled at master server.
ok |
|
|
Awesome work, congrats on your first merged pull request! |
…a Dinky job and passes date parameters, the actual values are not being replaced (apache#18080)
3 participants
Was this PR generated or assisted by AI?
Purpose of the pull request
fix #17943
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)
Pull Request Notice
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md