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

optimize code for restore exists resources #5293

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

cleverhu
Copy link
Contributor

@cleverhu cleverhu commented Sep 6, 2022

Thank you for contributing to Velero!

Please add a summary of your change

Optimize the previous logic and handle errors uniformly. If the resource exists but cannot be found, the 'get' error will be returned; otherwise, the creation error will be returned.

Does your change fix a particular issue?

Fixes #5288

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@cleverhu
Copy link
Contributor Author

cleverhu commented Sep 6, 2022

@sseago I think the logic modified now is equivalent to that before. Can you help me see what is wrong with my understanding.

@sseago
Copy link
Collaborator

sseago commented Sep 6, 2022

The biggest difference with this version is that in the "object doesn't exist" case, instead of adding restoreErr as a restore error, we add err -- so instead of the error returned by the create call, we're recording the "not found" error returned by the get call.

@cleverhu cleverhu force-pushed the optimize-restore branch 2 times, most recently from 2c78aff to a8d51b1 Compare September 6, 2022 15:25
@cleverhu
Copy link
Contributor Author

cleverhu commented Sep 6, 2022

The biggest difference with this version is that in the "object doesn't exist" case, instead of adding restoreErr as a restore error, we add err -- so instead of the error returned by the create call, we're recording the "not found" error returned by the get call.

@sseago I submitted a new version. What do you think of this version? I always feel a little strange about the previous writing.

@cleverhu cleverhu force-pushed the optimize-restore branch 3 times, most recently from 395638a to c0a3440 Compare September 6, 2022 16:12
@cleverhu cleverhu changed the title [DRAFT]trim isAlreadyExistsError for restore optimize code for restore exists resources Sep 6, 2022
@cleverhu cleverhu requested review from shubham-pampattiwar and removed request for dsu-igeek and blackpiglet September 6, 2022 19:57
@cleverhu
Copy link
Contributor Author

cleverhu commented Sep 6, 2022

@dsu-igeek @blackpiglet I'm sorry,I clicked the wrong button. I want @shubham-pampattiwar to help me review again instead of cancelling your review request. /PTAL

@blackpiglet blackpiglet requested a review from sseago September 7, 2022 06:15
@sseago sseago merged commit be0a1cf into vmware-tanzu:main Sep 8, 2022
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.

Velero restore partially failed as default priorityClass
3 participants