Skip to content

include db engine as implemented class #455

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 30, 2019
Merged

include db engine as implemented class #455

merged 1 commit into from
May 30, 2019

Conversation

drewmullen
Copy link
Member

@drewmullen drewmullen commented May 29, 2019 •
edited
Loading

database secrets engine wasnt enabled though the code was included. probably a good example for why we need unit tests :)

also - @jeffwecan may want to investigate why the error said '"%s" auth method class instead of database secret engine class

fix for #454

@codecov-io
Copy link

codecov-io commented May 29, 2019 •
edited
Loading

Codecov Report

Merging #455 into develop will increase coverage by 0.53%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           develop   #455      +/-   ##
=========================================
+ Coverage    87.47%    88%   +0.53%     
=========================================
  Files           50     50              
  Lines         2658   2659       +1     
=========================================
+ Hits          2325   2340      +15     
+ Misses         333    319      -14
Impacted Files Coverage Δ
hvac/api/secrets_engines/__init__.py 100% <100%> (ø) ⬆️
hvac/api/secrets_engines/database.py 35.89% <0%> (+35.89%) ⬆️

@jeffwecan
Copy link
Member

Ah good catch! My bad for missing that bit in the initial PR (the fact it was easy enough to miss is more fodder for #434 imho ;) ). The '"%s" auth method class is just a copy/paste carry over that should have been updated to state secrets engine class. If you wanna grab that fix here as well I would be down!

@jeffwecan jeffwecan added this to the 0.9.2 milestone May 30, 2019
@jeffwecan jeffwecan added the database database secrets engine label May 30, 2019
@drewmullen
Copy link
Member Author

@jeffwecan so if i changed:

hvac/hvac/api/vault_api_category.py

Line 42 in 9ad27d0

raise NotImplementedError('"%s" auth method class not currently implemented.')

wouldnt that also change the error when its searching for all 3 implementation lists ('AuthMethods', 'SecretsEngines', 'SystemBackend',)? i was trying to figure that out yesterday

the change might be more significant but im still pretty new to inheritance, etc

@jeffwecan
Copy link
Member

Ah yeah okay, didn't realize that bit was on the base class. We can skip it for now if you want and address that in another PR if you'd like in that case.

@drewmullen
Copy link
Member Author

yeah that sounds good. im gonna add a couple more bits to the secrets.database class too. will try to get that in by the weekend

@jeffwecan jeffwecan merged commit e8ae13a into hvac:develop May 30, 2019
@drewmullen drewmullen mentioned this pull request Jun 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
database database secrets engine
Projects
None yet
Milestone
0.9.2
Development

Successfully merging this pull request may close these issues.
3 participants
@drewmullen @codecov-io @jeffwecan