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

Support pipe command #3692

Merged

Conversation

cheyang
Copy link
Collaborator

@cheyang cheyang commented Jan 14, 2024

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

fixes #XXXX

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (6f018a3) 64.46% compared to head (1cf3646) 64.48%.
Report is 21 commits behind head on master.

Files Patch % Lines
pkg/utils/shell_pipes.go 87.87% 8 Missing ⚠️
pkg/ddc/jindo/operations/base.go 0.00% 3 Missing ⚠️
pkg/ddc/jindofsx/operations/base.go 0.00% 3 Missing ⚠️
pkg/ddc/juicefs/operations/base.go 0.00% 3 Missing ⚠️
pkg/ddc/thin/operations/base.go 0.00% 3 Missing ⚠️
pkg/ddc/alluxio/operations/base.go 33.33% 1 Missing and 1 partial ⚠️
pkg/ddc/goosefs/operations/base.go 33.33% 1 Missing and 1 partial ⚠️
pkg/utils/helm/helm.go 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3692      +/-   ##
==========================================
+ Coverage   64.46%   64.48%   +0.01%     
==========================================
  Files         471      472       +1     
  Lines       28153    28239      +86     
==========================================
+ Hits        18149    18209      +60     
- Misses       7848     7871      +23     
- Partials     2156     2159       +3     

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

Copy link

sonarcloud bot commented Jan 17, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
6.5% Duplication on New Code

See analysis details on SonarCloud

@cheyang cheyang changed the title Support safe pipeline bash Support pipe command Jan 17, 2024
@cheyang
Copy link
Collaborator Author

cheyang commented Jan 18, 2024

/test fluid-e2e

Copy link
Member

@TrafalgarZZZ TrafalgarZZZ left a comment

Choose a reason for hiding this comment

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

Merge it now, I will add a new PR to fix this.
/lgtm

}
})
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add a new empty line.

@@ -132,7 +132,11 @@ func GetChartVersion(chart string) (version string, err error) {
"|", "grep", "version:"}
log.V(1).Info("Exec bash -c", "args", args)

cmd := exec.Command("bash", "-c", strings.Join(args, " "))
// cmd := exec.Command("bash", "-c", strings.Join(args, " "))
cmd, err := utils.PipeCommand("bash", "-c", strings.Join(args, " "))
Copy link
Member

Choose a reason for hiding this comment

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

It seems that this is unnecessary to use pipe. We can use ddc-helm inspect chart <chart> and scan the returned string. I would like to change this in a next PR.

Copy link

fluid-e2e-bot bot commented Jan 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TrafalgarZZZ

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fluid-e2e-bot fluid-e2e-bot bot merged commit 5a1bb89 into fluid-cloudnative:master Jan 19, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants