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

Remove storage calls from local.Driver.alterModules #184

Conversation

davis-haba
Copy link
Contributor

Remove UpsertPolicy/DeletePolicy from local.driver.altermodules

#156

@willbeason willbeason self-requested a review January 19, 2022 19:49
@willbeason
Copy link
Member

FYI - you'll need to add DCO for this commit (the failure has the exact command to fix it for this).

In the long term, you may want to set up a commit hook that automatically appends the signature line so you never forget DCO.

Signed-off-by: davis-haba <davishaba@google.com>
@davis-haba davis-haba force-pushed the remove-storage-calls-from-local-driver-alterModules branch from 3bc39c9 to 4e8c239 Compare January 19, 2022 20:04
@davis-haba
Copy link
Contributor Author

davis-haba commented Jan 19, 2022

Thanks @willbeason. I updated the commit with the signature.

Copy link
Member

@willbeason willbeason left a comment

Choose a reason for hiding this comment

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

Great!

@sozercan
Copy link
Member

@davis-haba looks like unit tests are failing

…es to storage

Signed-off-by: davis-haba <davishaba@google.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2022

Codecov Report

Merging #184 (ac8fe07) into master (b0196c3) will decrease coverage by 0.14%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
- Coverage   45.70%   45.56%   -0.15%     
==========================================
  Files          59       59              
  Lines        2866     2853      -13     
==========================================
- Hits         1310     1300      -10     
+ Misses       1309     1307       -2     
+ Partials      247      246       -1     
Flag Coverage Δ
unittests 45.56% <ø> (-0.15%) ⬇️

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

Impacted Files Coverage Δ
...works/constraint/pkg/client/drivers/local/local.go 64.66% <0.00%> (-0.49%) ⬇️

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 b0196c3...ac8fe07. Read the comment docs.

Signed-off-by: davis-haba <davishaba@google.com>
Signed-off-by: davis-haba <davishaba@google.com>
@davis-haba
Copy link
Contributor Author

davis-haba commented Jan 20, 2022

@davis-haba looks like unit tests are failing

Thanks @sozercan. The tests were failing due to a test which mocked storage errors in local.driver.PutModule. Since we no longer write to storage during PutModule, I deleted the test.

@willbeason
Copy link
Member

Merging. I'll update my current test PR in gatekeeper so it includes these changes, to see if it catches any problems.

@willbeason willbeason merged commit 5d06ded into open-policy-agent:master Jan 21, 2022
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.

None yet

4 participants