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

split the result filed to metadata and inspect #3043

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

chengjoey
Copy link
Contributor

What type of this PR

Add one of the following kinds:
/kind bugfix

What this PR does / why we need it:

split the result filed to metadata and inspect, update metadata only when task callback

Which issue(s) this PR fixes:

Specified Reviewers:

/assign @your-reviewer

ChangeLog

Need cherry-pick to release versions?

Add comment like /cherry-pick release/1.0 when this PR is merged.

For details on the cherry pick process, see the cherry pick requests section under CONTRIBUTING.md.

@chengjoey chengjoey added bug pipeline pipeline service labels Nov 12, 2021
@chengjoey chengjoey force-pushed the fix/pipeline-metadata-missed branch from 9937bd7 to f52119f Compare November 15, 2021 02:35
@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #3043 (f1c18b8) into master (e9ffe30) will increase coverage by 0.02%.
The diff coverage is 37.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3043      +/-   ##
==========================================
+ Coverage   17.37%   17.40%   +0.02%     
==========================================
  Files        1361     1361              
  Lines      140447   140449       +2     
==========================================
+ Hits        24401    24441      +40     
+ Misses     113500   113474      -26     
+ Partials     2546     2534      -12     
Impacted Files Coverage Δ
apistructs/pipeline_task.go 13.88% <0.00%> (-56.95%) ⬇️
...ine/pipengine/reconciler/reconcile_data_process.go 1.36% <0.00%> (ø)
...pipeline/pipengine/reconciler/taskrun/framework.go 0.00% <0.00%> (ø)
...ine/pipengine/reconciler/taskrun/taskop/prepare.go 1.33% <0.00%> (+0.66%) ⬆️
...es/pipeline/pipengine/reconciler/taskrun/update.go 0.00% <0.00%> (ø)
modules/pipeline/services/pipelinesvc/detail.go 15.05% <0.00%> (-0.06%) ⬇️
modules/pipeline/pipengine/reconciler/snippet.go 23.07% <25.00%> (+14.56%) ⬆️
modules/pipeline/services/pipelinesvc/callback.go 37.80% <41.17%> (+35.02%) ⬆️
modules/pipeline/spec/pipeline_task.go 64.70% <91.66%> (+38.73%) ⬆️
...pipeline/pipengine/reconciler/taskrun/task_loop.go 80.88% <100.00%> (ø)
... and 11 more

@chengjoey chengjoey force-pushed the fix/pipeline-metadata-missed branch 2 times, most recently from 817c577 to 0d670f0 Compare November 15, 2021 06:13
@chengjoey
Copy link
Contributor Author

image

convert to pipeline task dto:
image

@chengjoey chengjoey force-pushed the fix/pipeline-metadata-missed branch 3 times, most recently from 3e321a9 to 549a654 Compare November 16, 2021 03:46
@@ -0,0 +1 @@
ALTER TABLE `pipeline_tasks` ADD COLUMN `inspect` text COMMENT 'task的调度信息' AFTER `result`;
Copy link
Member

Choose a reason for hiding this comment

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

mediumtext

@@ -62,6 +62,7 @@ type TaskContainer struct {
ContainerID string `json:"containerID"`
}

// PipelineTaskResult spec.pipeline task only use metadata, task dto has all fields
type PipelineTaskResult struct {
Metadata Metadata `json:"metadata,omitempty"`
Errors []*PipelineTaskErrResponse `json:"errors,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

use embed struct

@@ -195,7 +203,7 @@ func (o orderedResponses) Len() int { return len(o) }
func (o orderedResponses) Less(i, j int) bool { return o[i].Ctx.EndTime.Before(o[j].Ctx.EndTime) }
func (o orderedResponses) Swap(i, j int) { o[i], o[j] = o[j], o[i] }

func (t *PipelineTaskResult) AppendError(newResponses ...*PipelineTaskErrResponse) []*PipelineTaskErrResponse {
func (t *PipelineTaskInspect) AppendError(newResponses ...*PipelineTaskErrResponse) []*PipelineTaskErrResponse {
Copy link
Member

Choose a reason for hiding this comment

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

use result is still ok

meta := latestTask.Result.Metadata
var meta apistructs.Metadata
if latestTask.Result != nil {
meta = (*latestTask.Result).Metadata
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 function for spec.task to get metadata.
judge nil each time is bad.

@chengjoey chengjoey force-pushed the fix/pipeline-metadata-missed branch from 549a654 to 57a87aa Compare November 19, 2021 10:51
@chengjoey chengjoey force-pushed the fix/pipeline-metadata-missed branch from 57a87aa to f1c18b8 Compare November 22, 2021 06:06
@sfwn sfwn merged commit 0966736 into erda-project:master Nov 22, 2021
@chengjoey chengjoey deleted the fix/pipeline-metadata-missed branch April 18, 2022 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug pipeline pipeline service
Development

Successfully merging this pull request may close these issues.

2 participants