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
Integrate opa-client-go lib and reduce excessive OPA logging #60
Conversation
0334ace
to
ef8c10a
Compare
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.
LGTM, but let's test it on dev to see if anything pops up.
60acebb
to
5416423
Compare
9880e59
to
a5dcfbb
Compare
@ilian2233 After resolving the CircleCI issues, I was able to deploy and test all scenarios on dev. Please review again, feel free to test on dev if you have the time to do so. I am ok to proceed with this after I receive the necessary approvels. |
3134eb7
to
3ed0526
Compare
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.
LGTM
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.
LGTM! Just need to change the circleci setup and we should be good to go.
e518417
to
3251293
Compare
@@ -138,7 +140,7 @@ func main() { | |||
Desc: "Open policy agent sidecar address", | |||
EnvVar: "OPEN_POLICY_AGENT_ADDRESS", | |||
}) | |||
openPolicyAgentPolicyPath := app.String(cli.StringOpt{ | |||
opaPolicyPath := app.String(cli.StringOpt{ |
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.
on a general level we can use the new way of getting env vars using caarlos0 library. You can check how we are doing it here for reference: https://github.com/Financial-Times/public-content-relation-api/blob/main/pkg/config/config.go
Example: Name string
env:"APP_NAME" envDefault:"public-content-relation-api"``
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.
Thanks for the approval of the PR and this suggestion, Toni, I will definitely check it out for my next code changes.
Description
What
Why
UPPSF-4874
Anything, in particular, you'd like to highlight to reviewers
Scope and particulars of this PR (Please tick all that apply)
DoD - Ensure all relevant tasks are completed before marking this PR as "Ready for review"
This Pull Request follows the rules described in our Pull Requests Guide