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

Added support for centralized dynamic config #24

Merged
merged 7 commits into from Jun 25, 2016

Conversation

kevsmith
Copy link
Member

Wanted to get review on the code early while I'm working on test coverage.


# Use managed dynamic configuration
# Requires dynamic_config_root
# Environment variable: RELAY_MANAGED_DYNAMIC_CONFIG
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll need to add this to the doc site: http://docs.operable.io/docs/relay-environment-variables

* Made the dynamic config refresh interval configurable
* Immediately update dynamic config when the DCU starts to reduce update
  propagation latencies
* Cleaned up interval reporting phrasing in general
defer dcu.refreshTimer.Reset(refreshInterval)
var envelope messages.DynamicConfigsResponseEnvelope
decoder := json.NewDecoder(bytes.NewReader(payload))
decoder.UseNumber()
Copy link
Collaborator

Choose a reason for hiding this comment

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

One day, we should try and pull all the JSON decoding into a module so we don't have to keep repeating this same song and dance... 😦 . I'm afraid one day we're going to add something new and forget about this little wrinkle.

func (dcu *DynamicConfigUpdater) loop() {
<-dcu.control
dcu.refreshTimer.Stop()
dcu.conn.Disconnect()
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does this loop?

@kevsmith kevsmith force-pushed the kevsmith/central-dyn-config branch from d775b90 to 5e6433a Compare June 25, 2016 14:55
@christophermaier
Copy link
Collaborator

👍

@kevsmith kevsmith force-pushed the kevsmith/central-dyn-config branch from ae58f69 to b54815a Compare June 25, 2016 15:19
@kevsmith kevsmith merged commit 628ce09 into master Jun 25, 2016
@kevsmith kevsmith removed the review label Jun 25, 2016
@kevsmith kevsmith deleted the kevsmith/central-dyn-config branch June 25, 2016 15:19
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

2 participants