-
Notifications
You must be signed in to change notification settings - Fork 112
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
[cleanup] move env vars out of /api folder #1681
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (65.51%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1681 +/- ##
=======================================
Coverage 49.49% 49.49%
=======================================
Files 218 218
Lines 21244 21244
=======================================
Hits 10515 10515
Misses 10182 10182
Partials 547 547
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
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 again for the effort! Left 2 non blocking comments
api/datadoghq/v2alpha1/envvar.go
Outdated
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.
I'd be in favor of fully removing this file now that it's empty
pkg/constants/envvar.go
Outdated
DDddURL = "DD_DD_URL" | ||
DDURL = "DD_URL" | ||
DDSite = "DD_SITE" | ||
DDLogLevel = "DD_LOG_LEVEL" |
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.
nit: Could possibly move DDLogLevel
over to internal/controller/datadogagent/override/envvar.go
as it's only used in internal/controller/datadogagent/override/
multiple times and once in internal/controller/testutils
where it could be simply replaced as a string DD_LOG_LEVEL
What does this PR do?
The environment variables in v2alpha1 aren't used in the CRDS. Moved the variables defined in /api to:
internal/controller/datadogagent/common/envvar.go
if they are used in the controller and in multiple featuresenvvar.go
file in a feature if it's used exclusively in one featureMotivation
We don't need to have the constants in /api if they aren't relevant to the CRDs. Moving the constants closer to where they are used makes the code easier to maintain.
Additional Notes
Anything else we should know when reviewing?
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
should have no effect, everything builds and runs as normal
Checklist
bug
,enhancement
,refactoring
,documentation
,tooling
, and/ordependencies
qa/skip-qa
label