[Fix-#6783] fix: #6783 switchVersion error (#6783)#6784
[Fix-#6783] fix: #6783 switchVersion error (#6783)#6784brave-lee merged 5 commits intoapache:devfrom zwZjut:#6783
Conversation
zwZjut
changed the title
|
@JinyLeeChina |
|
Hi @zwZjut , to target specific issue from PR, we recommend use keyword |
| TaskDefinitionLog taskDefinitionUpdate = taskDefinitionLogMapper.queryByDefinitionCodeAndVersion(taskCode, version); | ||
| taskDefinitionUpdate.setUserId(loginUser.getId()); | ||
| taskDefinitionUpdate.setUpdateTime(new Date()); | ||
| taskDefinitionUpdate.setId(taskDefinition.getId()); |
There was a problem hiding this comment.
I do not think directly set task definitions log id here. I think both task definition and task definition log id could be auto increase. Am I wrong @JinyLeeChina
There was a problem hiding this comment.
in my opinion , switchVersion is to set the version of t_ds_task_definition , which reference from t_ds_task_definition_log, let's see the metadata, version=6 id=8 in t_ds_task_definition, but version=6 id=22 in t_ds_task_definition_log, taskDefinitionMapper.updateById(t_ds_task_definition_log) will throw "Switch task definition version error" because id=22 not in t_ds_task_definition, or maybe I am wrong, but the problem I got when I call openapi "switchTaskDefinitionVersion"
There was a problem hiding this comment.
I go to our UI try to find out which place call service switchVersion but failed 😢 Did one know where we call it?
There was a problem hiding this comment.
Oh, I got your point. You means that function switchVersion it's get task definition from task_definition_log and recover the snapshot to task_definition? If so your change make sense.
But the total logic is odd, could anyone tell me where would call this function?
There was a problem hiding this comment.
I think it's not used yet, because task versions is not open to users now , I'am just testing all openapis by create httprequest to help myself get familiar with the code
There was a problem hiding this comment.
update t_ds_task_definition by taskDefinitionLog will never succeed. int switchVersion = taskDefinitionMapper.updateById(taskDefinitionLog); line 288 , TaskDefinitionServiceImpl.java id is differnet between t_ds_task_definition and t_ds_task_definition, you may see the metadata of picture and it's explanation above
There was a problem hiding this comment.
One line of code is missing here, taskDefinitionLog.setId(taskDefinition.getId()); , Can you submit a PR to fix it ？
There was a problem hiding this comment.
oh, I was careless, you have added.
There was a problem hiding this comment.
this pr is to fix what you mentioned , taskDefinitionUpdate.setId(taskDefinition.getId()); (line 294) and I just change the java obj name "taskDefinitionLog" to "taskDefinitionUpdate" is to underline that we get obj from t_ds_task_definition_log it to update t_ds_task_definition
zwZjut
changed the title
Codecov Report
@@ Coverage Diff @@
## dev #6784 +/- ##
============================================
- Coverage 41.79% 41.78% -0.02%
+ Complexity 3615 3611 -4
============================================
Files 641 641
Lines 25890 25891 +1
Branches 2795 2795
============================================
- Hits 10821 10818 -3
Misses 14094 14094
- Partials 975 979 +4
Continue to review full report at Codecov.
|
|
Kudos, SonarCloud Quality Gate passed! |
5 participants
Purpose of the pull request
#6783
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: