Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix args order in strings.Contains in es-rollover #3324

Merged
merged 2 commits into from
Oct 15, 2021

Conversation

pavolloffay
Copy link
Member

Closes #3319

If we do strings.Contains("resource_already_exists_exception", errorMap["type"].(string)),
we're basically doing "resource_already_exists_exception" == errorMap["type"].(string).
I would assume that we intended to test whether errorMap["type"] contains
"resource_already_exists_exception" as a substring, so the reverse strings.Contains order
is more applicable here.

Signed-off-by: Pavol Loffay p.loffay@gmail.com

Which problem is this PR solving?

Short description of the changes

If we do `strings.Contains("resource_already_exists_exception", errorMap["type"].(string))`,
we're basically doing `"resource_already_exists_exception" == errorMap["type"].(string)`.
I would assume that we intended to test whether `errorMap["type"]` contains
`"resource_already_exists_exception"` as a substring, so the reverse `strings.Contains` order
is more applicable here.

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Copy link
Contributor

@rubenvp8510 rubenvp8510 left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Oct 14, 2021

Codecov Report

Merging #3324 (7199205) into master (51ce980) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3324      +/-   ##
==========================================
+ Coverage   95.96%   96.00%   +0.03%     
==========================================
  Files         259      259              
  Lines       15434    15434              
==========================================
+ Hits        14811    14817       +6     
+ Misses        528      523       -5     
+ Partials       95       94       -1     
Impacted Files Coverage Δ
cmd/es-rollover/app/init/action.go 93.75% <100.00%> (ø)
cmd/query/app/server.go 95.58% <0.00%> (+1.47%) ⬆️
pkg/config/tlscfg/cert_watcher.go 94.73% <0.00%> (+2.10%) ⬆️
cmd/collector/app/server/zipkin.go 70.73% <0.00%> (+2.43%) ⬆️

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 51ce980...7199205. Read the comment docs.

@yurishkuro yurishkuro merged commit 15abdb4 into jaegertracing:master Oct 15, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants