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

fix(executor): Correctly surface error when resource is deleted during status checking #5675

Merged
merged 2 commits into from
Apr 15, 2021
Merged

fix(executor): Correctly surface error when resource is deleted during status checking #5675

merged 2 commits into from
Apr 15, 2021

Conversation

terrytangyuan
Copy link
Member

In the current implementation, strings.Contains(jsonString, "NotFound") block would never be reached since the error would be returned immediately if it's not transient. Also switched to use apierr.IsNotFound(err) which is more robust than string matching.

Signed-off-by: terrytangyuan terrytangyuan@gmail.com

Checklist:

@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #5675 (5400667) into master (eab122b) will increase coverage by 0.07%.
The diff coverage is 0.00%.

❗ Current head 5400667 differs from pull request most recent head 24c7299. Consider uploading reports for the commit 24c7299 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5675      +/-   ##
==========================================
+ Coverage   47.07%   47.15%   +0.07%     
==========================================
  Files         242      242              
  Lines       15135    15136       +1     
==========================================
+ Hits         7125     7137      +12     
+ Misses       7105     7093      -12     
- Partials      905      906       +1     
Impacted Files Coverage Δ
workflow/executor/resource.go 22.80% <0.00%> (-0.14%) ⬇️
cmd/argo/commands/get.go 56.95% <0.00%> (+0.64%) ⬆️
cmd/argoexec/commands/emissary.go 50.00% <0.00%> (+1.56%) ⬆️
server/workflow/workflow_server.go 42.73% <0.00%> (+2.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eab122b...24c7299. Read the comment docs.

@terrytangyuan
Copy link
Member Author

Codegen failed since install_kustomize.sh failed, which is unrelated.

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Please fix codegen

…g status checking

Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
@terrytangyuan
Copy link
Member Author

terrytangyuan commented Apr 15, 2021

@alexec It was a timeout/connection issue when installing kustomize on GitHub Actions. Re-running fixed it.

@alexec alexec merged commit 24ac725 into argoproj:master Apr 15, 2021
@terrytangyuan terrytangyuan deleted the executor-res-not-found branch April 15, 2021 18:37
@simster7 simster7 mentioned this pull request Apr 19, 2021
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants