-
Notifications
You must be signed in to change notification settings - Fork 133
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
ROX-18155: pg generic store: GetMany #6773
ROX-18155: pg generic store: GetMany #6773
Conversation
Images are ready for the commit at 569f2ba. To use with deploy scripts, first |
Images are ready for the commit at 42ab190. To use with deploy scripts, first |
6ea061a
to
30d04da
Compare
42ab190
to
91f09f5
Compare
30d04da
to
6182ccc
Compare
91f09f5
to
36679e0
Compare
6182ccc
to
4273cef
Compare
36679e0
to
efaad61
Compare
4273cef
to
309b1a3
Compare
efaad61
to
043cf9f
Compare
309b1a3
to
556711f
Compare
043cf9f
to
d8a82a8
Compare
556711f
to
5d56899
Compare
d8a82a8
to
a55c307
Compare
5d56899
to
573d29d
Compare
a55c307
to
2cffae6
Compare
/retest |
|
||
var sacQueryFilter *v1.Query | ||
if s.hasPermissionsChecker() { | ||
if ok, err := s.permissionChecker.GetAllowed(ctx); err != nil || !ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current state of existing PermissionChecker
implementation, it does not make any difference, as these will check for global read access to the underlying resource(s).
Nevertheless, the generated store code was calling GetManyAllowed
on the permission checker.
573d29d
to
61ff3c2
Compare
2cffae6
to
e17c60a
Compare
/retest |
Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready. |
e17c60a
to
569f2ba
Compare
@janisz: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Description
A detailed explanation of the changes in your PR.
Feel free to remove this section if it is overkill for your PR, and the title of your PR is sufficiently descriptive.
Checklist
If any of these don't apply, please comment below.
Testing Performed
TODO(replace-me)
Use this space to explain how you tested your PR, or, if you didn't test it, why
you did not do so. Valid reasons include, for example, "CI is sufficient",
"No testable changes". Feel free to attach JSON snippets, curl commands,
screenshots.
In addition to reviewing your code, reviewers must also review your testing
instructions and make sure they are sufficient.