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

Generate masterN.mesos record for current leader #169

Merged
merged 3 commits into from Jun 12, 2015
Merged

Generate masterN.mesos record for current leader #169

merged 3 commits into from Jun 12, 2015

Conversation

jdef
Copy link
Contributor

@jdef jdef commented Jun 11, 2015

Fixes #165

@jdef jdef added the PTAL label Jun 11, 2015
@tsenart tsenart changed the title fix #165 Generate masterN.mesos record for current leader Jun 11, 2015
}
if rtype == "A" {
val := rg.As[name]
rg.As[name] = append(val, host)
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of making this a nested map?

if _, ok := rg.records[rtype]; !ok {
    rg.records[rtype] = map[string][]string{}
}
rg.records[rtype][name] = append(rg.records[rtype][name], host)

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even better:

if _, ok := rg.records[rtype]; !ok {
    rg.records[rtype] = map[string]map[string]struct{}{}
}

if _, ok := rg.records[rtype][name]; !ok {
    rg.records[rtype][name] = map[string]struct{}{}
    rg.records[rtype][host] = map[string]struct{}{}
}

rg.records[rtype][name][host] = struct{}{}
rg.records[rtype][host][name] = struct{}{}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good suggestions. if the system wasn't about to undergo major re-implementation to support new features, i'd agree to an approach like this.

@tsenart tsenart assigned jdef and unassigned tsenart Jun 11, 2015
@kozyraki
Copy link
Contributor

@jdef @tsenart Gentlemen, is this good to go from your point of view? Cannot tell for sure.

@jdef
Copy link
Contributor Author

jdef commented Jun 11, 2015

Not ready for merge, needs additional work.

On Thu, Jun 11, 2015 at 1:38 PM, Christos Kozyrakis <
notifications@github.com> wrote:

@jdef https://github.com/jdef @tsenart https://github.com/tsenart
Gentlemen, is this good to go from your point of view? Cannot tell for
sure.


Reply to this email directly or view it on GitHub
#169 (comment).

@jdef jdef added PTAL and removed WIP labels Jun 12, 2015
@jdef jdef assigned tsenart and unassigned jdef Jun 12, 2015
@tsenart
Copy link
Contributor

tsenart commented Jun 12, 2015

LGTM in regards to the scope of the PR. Let's review the other suggestions after we have this through.

tsenart added a commit that referenced this pull request Jun 12, 2015
Generate masterN.mesos record for current leader
@tsenart tsenart merged commit d7a0422 into master Jun 12, 2015
@jdef jdef deleted the fix-165 branch June 14, 2015 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants