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

[x/config] Deprecate src/x/config/listenaddress #2017

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

andrewmains12
Copy link
Contributor

What this PR does / why we need it: Given #2016, we no longer need a homebrew means of resolving ports from the environment. Users who want this functionality can now do:

listenAddress:
   value: 0.0.0.0:${MY_PORT}

Given sufficient documentation (hopefully provided), this is imo simpler and easier to grok behavior-wise than the homebrewed solution we were using before.

The change should be backwards compatible; anyone using type: environment will still have their
or

listenAddress:
   value: ${MY_ADRR}:${MY_PORT}

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:

Port/host environment resolution for query and m3collector configs can now be done using config environment interpolation in go.uber.org/config (see config/README.md for
details). Instead of:

    listenAddress:
      type: environment
      envVarListenPort: MY_PORT_VAR

 one can do:

    listenAddress:
      value: 0.0.0.0:${MY_PORT_VAR}

The old mechanism will continue to work, but will log a deprecation notice.

Does this PR require updating code package or user-facing documentation?:

Updated listenaddress package documentation with the deprecation notice.

config/README.md Outdated Show resolved Hide resolved
config/README.md Outdated
```

and an environment of `MY_ENV_VAR=bar`, we will load:
Copy link
Collaborator

Choose a reason for hiding this comment

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

we will load -> the interpolated YAML will be:

src/x/config/config.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 22, 2019

Codecov Report

Merging #2017 into master will decrease coverage by 3.3%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2017     +/-   ##
=========================================
- Coverage    63.1%    59.7%   -3.4%     
=========================================
  Files        1079     1105     +26     
  Lines      105667   104676    -991     
=========================================
- Hits        66679    62519   -4160     
- Misses      34816    37970   +3154     
- Partials     4172     4187     +15
Flag Coverage Δ
#aggregator 66.1% <ø> (+11.7%) ⬆️
#cluster 55.3% <ø> (-4.8%) ⬇️
#collector 8.9% <ø> (-41.6%) ⬇️
#dbnode 61.9% <ø> (+0.2%) ⬆️
#m3em 60.9% <ø> (+26.4%) ⬆️
#m3ninx 54.5% <ø> (-3%) ⬇️
#m3nsch 51.1% <ø> (ø) ⬆️
#metrics 21.7% <ø> (-31.2%) ⬇️
#msg 72.9% <ø> (+28.1%) ⬆️
#query 57% <ø> (-10.2%) ⬇️
#x 61.7% <100%> (-2.6%) ⬇️

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 9de19aa...6363f16. Read the comment docs.

@andrewmains12 andrewmains12 force-pushed the amains/config/deprecate_listen_addr branch from c181db8 to 6363f16 Compare October 24, 2019 18:29
@andrewmains12 andrewmains12 changed the base branch from amains/config/uber_config to master October 24, 2019 18:29
@andrewmains12 andrewmains12 merged commit de7f9ea into master Oct 24, 2019
@justinjc justinjc deleted the amains/config/deprecate_listen_addr branch November 27, 2019 20:53
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.

2 participants