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 KeyError when comparing datasource without basicAuth #316

Merged
merged 3 commits into from
Nov 5, 2023

Conversation

LiMuBei
Copy link
Contributor

@LiMuBei LiMuBei commented Oct 12, 2023

If there is no check for its presence, this will fail if a datasource is not configured with basic auth at all. In that case it would try to remove an entry from the dict that is not there resulting in a KeyError.

SUMMARY

When comparing the existing datasource with the should be state, basicAuth is handled incorrectly in the case when the datasource is not configured with basicAuth at all. This case would result in a KeyError which this change fixes.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

grafana_datasource.py

See also #248

@LiMuBei LiMuBei requested review from rrey and seuf as code owners October 12, 2023 19:00
@seuf
Copy link
Collaborator

seuf commented Oct 23, 2023

HEllo @LiMuBei . Regarding the checls failure, you need to add a changelog fragment so the tests can pass

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

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

Comparison is base (7ab900f) 71.07% compared to head (b910e81) 71.03%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #316      +/-   ##
==========================================
- Coverage   71.07%   71.03%   -0.04%     
==========================================
  Files          18       18              
  Lines        1853     1854       +1     
  Branches      318      319       +1     
==========================================
  Hits         1317     1317              
  Misses        396      396              
- Partials      140      141       +1     
Flag Coverage Δ
integration 69.74% <50.00%> (-0.07%) ⬇️
sanity 23.78% <0.00%> (-0.02%) ⬇️
units 65.59% <0.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
plugins/modules/grafana_datasource.py 76.71% <50.00%> (-0.41%) ⬇️

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

@LiMuBei
Copy link
Contributor Author

LiMuBei commented Nov 2, 2023

HEllo @LiMuBei . Regarding the checls failure, you need to add a changelog fragment so the tests can pass

Hi. I added a fragment.

Alexander Kasper and others added 3 commits November 5, 2023 00:53
If there is no check for its presence, this will fail if a datasource is not configured with basic auth at all.
In that case it would try to remove an entry from the dict that is not there resulting in a KeyError.
@rrey rrey merged commit b2efe0a into ansible-collections:main Nov 5, 2023
17 of 19 checks passed
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.

3 participants