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

cephfs: use string const for cli error comparison #1383

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Aug 17, 2020

we should not return the CLI errors in GRPC errors, we need to return proper readable error messages
to the user for better understanding and better debugging.

updates #1242

Signed-off-by: Madhu Rajanna madhupr007@gmail.com

@Madhu-1 Madhu-1 requested review from humblec and nixpanic and removed request for humblec August 17, 2020 06:04
@humblec
Copy link
Collaborator

humblec commented Aug 17, 2020

Thanks @Madhu-1

humblec
humblec previously approved these changes Aug 17, 2020
@humblec humblec added this to the release-3.2.0 milestone Aug 17, 2020
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 17, 2020

/retest all

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 17, 2020

/test ci/centos/mini-e2e-helm/k8s-1.17.8

nixpanic
nixpanic previously approved these changes Aug 17, 2020
@nixpanic
Copy link
Member

/retest ci/centos/mini-e2e

@nixpanic
Copy link
Member

/retest ci/centos/mini-e2e-helm/k8s-1.17.8

@nixpanic
Copy link
Member

/retest commitlint

@mergify
Copy link
Contributor

mergify bot commented Aug 18, 2020

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@mergify mergify bot dismissed stale reviews from humblec and nixpanic August 18, 2020 06:17

Pull request has been modified.

@Madhu-1 Madhu-1 requested review from nixpanic and humblec August 18, 2020 06:17
Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

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

Does this really fix, and therefore close #1242? It suggests everythin for the issue has been addressed in previous PRs. I do not think that is true (or the issue is not up to date). Please use Updates: #1242 instead.

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 18, 2020

Does this really fix, and therefore close #1242? It suggests everythin for the issue has been addressed in previous PRs. I do not think that is true (or the issue is not up to date). Please use Updates: #1242 instead.

done. please note that the issue needs to be closed manually as it was the last pending item in issue list

@Madhu-1 Madhu-1 requested a review from nixpanic August 18, 2020 06:33
@Madhu-1 Madhu-1 added the ready-to-merge This PR is ready to be merged and it doesn't need second review (backports only) label Aug 19, 2020
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 19, 2020

added ready-to-merge as @humblec already approved it

we should not return the CLI errors in GRPC errors
we need to return proper readable error messages
to the user for better understanding and better
debugging.

updates ceph#1242

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cephfs Issues related to CephFS ready-to-merge This PR is ready to be merged and it doesn't need second review (backports only)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants